[MCP] Redact environment variable values in /mcp and mcp get (#5648)
Fixes #5524
This commit is contained in:
@@ -523,10 +523,12 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
|
|||||||
.map(|entry| entry.auth_status)
|
.map(|entry| entry.auth_status)
|
||||||
.unwrap_or(McpAuthStatus::Unsupported)
|
.unwrap_or(McpAuthStatus::Unsupported)
|
||||||
.to_string();
|
.to_string();
|
||||||
|
let bearer_token_display =
|
||||||
|
bearer_token_env_var.as_deref().unwrap_or("-").to_string();
|
||||||
http_rows.push([
|
http_rows.push([
|
||||||
name.clone(),
|
name.clone(),
|
||||||
url.clone(),
|
url.clone(),
|
||||||
bearer_token_env_var.clone().unwrap_or("-".to_string()),
|
bearer_token_display,
|
||||||
status,
|
status,
|
||||||
auth_status,
|
auth_status,
|
||||||
]);
|
]);
|
||||||
@@ -752,15 +754,15 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
|
|||||||
} => {
|
} => {
|
||||||
println!(" transport: streamable_http");
|
println!(" transport: streamable_http");
|
||||||
println!(" url: {url}");
|
println!(" url: {url}");
|
||||||
let env_var = bearer_token_env_var.as_deref().unwrap_or("-");
|
let bearer_token_display = bearer_token_env_var.as_deref().unwrap_or("-");
|
||||||
println!(" bearer_token_env_var: {env_var}");
|
println!(" bearer_token_env_var: {bearer_token_display}");
|
||||||
let headers_display = match http_headers {
|
let headers_display = match http_headers {
|
||||||
Some(map) if !map.is_empty() => {
|
Some(map) if !map.is_empty() => {
|
||||||
let mut pairs: Vec<_> = map.iter().collect();
|
let mut pairs: Vec<_> = map.iter().collect();
|
||||||
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||||
pairs
|
pairs
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|(k, v)| format!("{k}={v}"))
|
.map(|(k, _)| format!("{k}=*****"))
|
||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
.join(", ")
|
.join(", ")
|
||||||
}
|
}
|
||||||
@@ -773,7 +775,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
|
|||||||
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||||
pairs
|
pairs
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|(k, v)| format!("{k}={v}"))
|
.map(|(k, var)| format!("{k}={var}"))
|
||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
.join(", ")
|
.join(", ")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -68,9 +68,9 @@ async fn list_and_get_render_expected_output() -> Result<()> {
|
|||||||
assert!(stdout.contains("Name"));
|
assert!(stdout.contains("Name"));
|
||||||
assert!(stdout.contains("docs"));
|
assert!(stdout.contains("docs"));
|
||||||
assert!(stdout.contains("docs-server"));
|
assert!(stdout.contains("docs-server"));
|
||||||
assert!(stdout.contains("TOKEN=secret"));
|
assert!(stdout.contains("TOKEN=*****"));
|
||||||
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
|
assert!(stdout.contains("APP_TOKEN=*****"));
|
||||||
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
|
assert!(stdout.contains("WORKSPACE_ID=*****"));
|
||||||
assert!(stdout.contains("Status"));
|
assert!(stdout.contains("Status"));
|
||||||
assert!(stdout.contains("Auth"));
|
assert!(stdout.contains("Auth"));
|
||||||
assert!(stdout.contains("enabled"));
|
assert!(stdout.contains("enabled"));
|
||||||
@@ -119,9 +119,9 @@ async fn list_and_get_render_expected_output() -> Result<()> {
|
|||||||
assert!(stdout.contains("transport: stdio"));
|
assert!(stdout.contains("transport: stdio"));
|
||||||
assert!(stdout.contains("command: docs-server"));
|
assert!(stdout.contains("command: docs-server"));
|
||||||
assert!(stdout.contains("args: --port 4000"));
|
assert!(stdout.contains("args: --port 4000"));
|
||||||
assert!(stdout.contains("env: TOKEN=secret"));
|
assert!(stdout.contains("env: TOKEN=*****"));
|
||||||
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
|
assert!(stdout.contains("APP_TOKEN=*****"));
|
||||||
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
|
assert!(stdout.contains("WORKSPACE_ID=*****"));
|
||||||
assert!(stdout.contains("enabled: true"));
|
assert!(stdout.contains("enabled: true"));
|
||||||
assert!(stdout.contains("remove: codex mcp remove docs"));
|
assert!(stdout.contains("remove: codex mcp remove docs"));
|
||||||
|
|
||||||
|
|||||||
@@ -6,15 +6,11 @@ pub fn format_env_display(env: Option<&HashMap<String, String>>, env_vars: &[Str
|
|||||||
if let Some(map) = env {
|
if let Some(map) = env {
|
||||||
let mut pairs: Vec<_> = map.iter().collect();
|
let mut pairs: Vec<_> = map.iter().collect();
|
||||||
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||||
parts.extend(
|
parts.extend(pairs.into_iter().map(|(key, _)| format!("{key}=*****")));
|
||||||
pairs
|
|
||||||
.into_iter()
|
|
||||||
.map(|(key, value)| format!("{key}={value}")),
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if !env_vars.is_empty() {
|
if !env_vars.is_empty() {
|
||||||
parts.extend(env_vars.iter().map(|var| format!("{var}=${var}")));
|
parts.extend(env_vars.iter().map(|var| format!("{var}=*****")));
|
||||||
}
|
}
|
||||||
|
|
||||||
if parts.is_empty() {
|
if parts.is_empty() {
|
||||||
@@ -42,14 +38,14 @@ mod tests {
|
|||||||
env.insert("B".to_string(), "two".to_string());
|
env.insert("B".to_string(), "two".to_string());
|
||||||
env.insert("A".to_string(), "one".to_string());
|
env.insert("A".to_string(), "one".to_string());
|
||||||
|
|
||||||
assert_eq!(format_env_display(Some(&env), &[]), "A=one, B=two");
|
assert_eq!(format_env_display(Some(&env), &[]), "A=*****, B=*****");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn formats_env_vars_with_dollar_prefix() {
|
fn formats_env_vars_with_dollar_prefix() {
|
||||||
let vars = vec!["TOKEN".to_string(), "PATH".to_string()];
|
let vars = vec!["TOKEN".to_string(), "PATH".to_string()];
|
||||||
|
|
||||||
assert_eq!(format_env_display(None, &vars), "TOKEN=$TOKEN, PATH=$PATH");
|
assert_eq!(format_env_display(None, &vars), "TOKEN=*****, PATH=*****");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -60,7 +56,7 @@ mod tests {
|
|||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
format_env_display(Some(&env), &vars),
|
format_env_display(Some(&env), &vars),
|
||||||
"HOME=/tmp, TOKEN=$TOKEN"
|
"HOME=*****, TOKEN=*****"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1047,7 +1047,10 @@ pub(crate) fn new_mcp_tools_output(
|
|||||||
return PlainHistoryCell { lines };
|
return PlainHistoryCell { lines };
|
||||||
}
|
}
|
||||||
|
|
||||||
for (server, cfg) in config.mcp_servers.iter() {
|
let mut servers: Vec<_> = config.mcp_servers.iter().collect();
|
||||||
|
servers.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||||
|
|
||||||
|
for (server, cfg) in servers {
|
||||||
let prefix = format!("mcp__{server}__");
|
let prefix = format!("mcp__{server}__");
|
||||||
let mut names: Vec<String> = tools
|
let mut names: Vec<String> = tools
|
||||||
.keys()
|
.keys()
|
||||||
@@ -1111,7 +1114,7 @@ pub(crate) fn new_mcp_tools_output(
|
|||||||
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||||
let display = pairs
|
let display = pairs
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|(name, value)| format!("{name}={value}"))
|
.map(|(name, _)| format!("{name}=*****"))
|
||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
.join(", ");
|
.join(", ");
|
||||||
lines.push(vec![" • HTTP headers: ".into(), display.into()].into());
|
lines.push(vec![" • HTTP headers: ".into(), display.into()].into());
|
||||||
@@ -1123,7 +1126,7 @@ pub(crate) fn new_mcp_tools_output(
|
|||||||
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||||
let display = pairs
|
let display = pairs
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|(name, env_var)| format!("{name}={env_var}"))
|
.map(|(name, var)| format!("{name}={var}"))
|
||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
.join(", ");
|
.join(", ");
|
||||||
lines.push(vec![" • Env HTTP headers: ".into(), display.into()].into());
|
lines.push(vec![" • Env HTTP headers: ".into(), display.into()].into());
|
||||||
@@ -1415,14 +1418,20 @@ mod tests {
|
|||||||
use codex_core::config::Config;
|
use codex_core::config::Config;
|
||||||
use codex_core::config::ConfigOverrides;
|
use codex_core::config::ConfigOverrides;
|
||||||
use codex_core::config::ConfigToml;
|
use codex_core::config::ConfigToml;
|
||||||
|
use codex_core::config_types::McpServerConfig;
|
||||||
|
use codex_core::config_types::McpServerTransportConfig;
|
||||||
|
use codex_core::protocol::McpAuthStatus;
|
||||||
use codex_protocol::parse_command::ParsedCommand;
|
use codex_protocol::parse_command::ParsedCommand;
|
||||||
use dirs::home_dir;
|
use dirs::home_dir;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
|
use std::collections::HashMap;
|
||||||
|
|
||||||
use mcp_types::CallToolResult;
|
use mcp_types::CallToolResult;
|
||||||
use mcp_types::ContentBlock;
|
use mcp_types::ContentBlock;
|
||||||
use mcp_types::TextContent;
|
use mcp_types::TextContent;
|
||||||
|
use mcp_types::Tool;
|
||||||
|
use mcp_types::ToolInputSchema;
|
||||||
|
|
||||||
fn test_config() -> Config {
|
fn test_config() -> Config {
|
||||||
Config::load_from_base_config_with_overrides(
|
Config::load_from_base_config_with_overrides(
|
||||||
@@ -1449,6 +1458,91 @@ mod tests {
|
|||||||
render_lines(&cell.transcript_lines(u16::MAX))
|
render_lines(&cell.transcript_lines(u16::MAX))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn mcp_tools_output_masks_sensitive_values() {
|
||||||
|
let mut config = test_config();
|
||||||
|
let mut env = HashMap::new();
|
||||||
|
env.insert("TOKEN".to_string(), "secret".to_string());
|
||||||
|
let stdio_config = McpServerConfig {
|
||||||
|
transport: McpServerTransportConfig::Stdio {
|
||||||
|
command: "docs-server".to_string(),
|
||||||
|
args: vec![],
|
||||||
|
env: Some(env),
|
||||||
|
env_vars: vec!["APP_TOKEN".to_string()],
|
||||||
|
cwd: None,
|
||||||
|
},
|
||||||
|
enabled: true,
|
||||||
|
startup_timeout_sec: None,
|
||||||
|
tool_timeout_sec: None,
|
||||||
|
enabled_tools: None,
|
||||||
|
disabled_tools: None,
|
||||||
|
};
|
||||||
|
config.mcp_servers.insert("docs".to_string(), stdio_config);
|
||||||
|
|
||||||
|
let mut headers = HashMap::new();
|
||||||
|
headers.insert("Authorization".to_string(), "Bearer secret".to_string());
|
||||||
|
let mut env_headers = HashMap::new();
|
||||||
|
env_headers.insert("X-API-Key".to_string(), "API_KEY_ENV".to_string());
|
||||||
|
let http_config = McpServerConfig {
|
||||||
|
transport: McpServerTransportConfig::StreamableHttp {
|
||||||
|
url: "https://example.com/mcp".to_string(),
|
||||||
|
bearer_token_env_var: Some("MCP_TOKEN".to_string()),
|
||||||
|
http_headers: Some(headers),
|
||||||
|
env_http_headers: Some(env_headers),
|
||||||
|
},
|
||||||
|
enabled: true,
|
||||||
|
startup_timeout_sec: None,
|
||||||
|
tool_timeout_sec: None,
|
||||||
|
enabled_tools: None,
|
||||||
|
disabled_tools: None,
|
||||||
|
};
|
||||||
|
config.mcp_servers.insert("http".to_string(), http_config);
|
||||||
|
|
||||||
|
let mut tools: HashMap<String, Tool> = HashMap::new();
|
||||||
|
tools.insert(
|
||||||
|
"mcp__docs__list".to_string(),
|
||||||
|
Tool {
|
||||||
|
annotations: None,
|
||||||
|
description: None,
|
||||||
|
input_schema: ToolInputSchema {
|
||||||
|
properties: None,
|
||||||
|
required: None,
|
||||||
|
r#type: "object".to_string(),
|
||||||
|
},
|
||||||
|
name: "list".to_string(),
|
||||||
|
output_schema: None,
|
||||||
|
title: None,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
tools.insert(
|
||||||
|
"mcp__http__ping".to_string(),
|
||||||
|
Tool {
|
||||||
|
annotations: None,
|
||||||
|
description: None,
|
||||||
|
input_schema: ToolInputSchema {
|
||||||
|
properties: None,
|
||||||
|
required: None,
|
||||||
|
r#type: "object".to_string(),
|
||||||
|
},
|
||||||
|
name: "ping".to_string(),
|
||||||
|
output_schema: None,
|
||||||
|
title: None,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
let auth_statuses: HashMap<String, McpAuthStatus> = HashMap::new();
|
||||||
|
let cell = new_mcp_tools_output(
|
||||||
|
&config,
|
||||||
|
tools,
|
||||||
|
HashMap::new(),
|
||||||
|
HashMap::new(),
|
||||||
|
&auth_statuses,
|
||||||
|
);
|
||||||
|
let rendered = render_lines(&cell.display_lines(120)).join("\n");
|
||||||
|
|
||||||
|
insta::assert_snapshot!(rendered);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn empty_agent_message_cell_transcript() {
|
fn empty_agent_message_cell_transcript() {
|
||||||
let cell = AgentMessageCell::new(vec![Line::default()], false);
|
let cell = AgentMessageCell::new(vec![Line::default()], false);
|
||||||
|
|||||||
@@ -0,0 +1,27 @@
|
|||||||
|
---
|
||||||
|
source: tui/src/history_cell.rs
|
||||||
|
assertion_line: 1540
|
||||||
|
expression: rendered
|
||||||
|
---
|
||||||
|
/mcp
|
||||||
|
|
||||||
|
🔌 MCP Tools
|
||||||
|
|
||||||
|
• docs
|
||||||
|
• Status: enabled
|
||||||
|
• Auth: Unsupported
|
||||||
|
• Command: docs-server
|
||||||
|
• Env: TOKEN=*****, APP_TOKEN=*****
|
||||||
|
• Tools: list
|
||||||
|
• Resources: (none)
|
||||||
|
• Resource templates: (none)
|
||||||
|
|
||||||
|
• http
|
||||||
|
• Status: enabled
|
||||||
|
• Auth: Unsupported
|
||||||
|
• URL: https://example.com/mcp
|
||||||
|
• HTTP headers: Authorization=*****
|
||||||
|
• Env HTTP headers: X-API-Key=API_KEY_ENV
|
||||||
|
• Tools: ping
|
||||||
|
• Resources: (none)
|
||||||
|
• Resource templates: (none)
|
||||||
Reference in New Issue
Block a user