From fcd197d5962347db2abc21043b806fb0b560820c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 28 Jul 2025 08:52:18 -0700 Subject: [PATCH] fix: use std::env::args_os instead of std::env::args (#1698) Apparently `std::env::args()` will panic during iteration if any argument to the process is not valid Unicode: https://doc.rust-lang.org/std/env/fn.args.html Let's avoid the risk and just go with `std::env::args_os()`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1698). * #1705 * #1703 * #1702 * __->__ #1698 * #1697 --- codex-rs/arg0/src/lib.rs | 2 +- codex-rs/core/src/mcp_connection_manager.rs | 8 +++++++- codex-rs/mcp-client/src/main.rs | 3 ++- codex-rs/mcp-client/src/mcp_client.rs | 5 +++-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index 86b98c7d..624583b8 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -30,7 +30,7 @@ where Fut: Future>, { // Determine if we were invoked via the special alias. - let argv0 = std::env::args().next().unwrap_or_default(); + let argv0 = std::env::args_os().next().unwrap_or_default(); let exe_name = Path::new(&argv0) .file_name() .and_then(|s| s.to_str()) diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 886e4f8b..2e33c875 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -8,6 +8,7 @@ use std::collections::HashMap; use std::collections::HashSet; +use std::ffi::OsString; use std::time::Duration; use anyhow::Context; @@ -127,7 +128,12 @@ impl McpConnectionManager { join_set.spawn(async move { let McpServerConfig { command, args, env } = cfg; - let client_res = McpClient::new_stdio_client(command, args, env).await; + let client_res = McpClient::new_stdio_client( + command.into(), + args.into_iter().map(OsString::from).collect(), + env, + ) + .await; match client_res { Ok(client) => { // Initialize the client. diff --git a/codex-rs/mcp-client/src/main.rs b/codex-rs/mcp-client/src/main.rs index 8d671b83..10cfe389 100644 --- a/codex-rs/mcp-client/src/main.rs +++ b/codex-rs/mcp-client/src/main.rs @@ -10,6 +10,7 @@ //! program. The utility connects, issues a `tools/list` request and prints the //! server's response as pretty JSON. +use std::ffi::OsString; use std::time::Duration; use anyhow::Context; @@ -37,7 +38,7 @@ async fn main() -> Result<()> { .try_init(); // Collect command-line arguments excluding the program name itself. - let mut args: Vec = std::env::args().skip(1).collect(); + let mut args: Vec = std::env::args_os().skip(1).collect(); if args.is_empty() || args[0] == "--help" || args[0] == "-h" { eprintln!("Usage: mcp-client [args..]\n\nExample: mcp-client codex-mcp-server"); diff --git a/codex-rs/mcp-client/src/mcp_client.rs b/codex-rs/mcp-client/src/mcp_client.rs index 6a9111e6..084d0bf4 100644 --- a/codex-rs/mcp-client/src/mcp_client.rs +++ b/codex-rs/mcp-client/src/mcp_client.rs @@ -12,6 +12,7 @@ //! issue requests and receive strongly-typed results. use std::collections::HashMap; +use std::ffi::OsString; use std::sync::Arc; use std::sync::atomic::AtomicI64; use std::sync::atomic::Ordering; @@ -82,8 +83,8 @@ impl McpClient { /// Caller is responsible for sending the `initialize` request. See /// [`initialize`](Self::initialize) for details. pub async fn new_stdio_client( - program: String, - args: Vec, + program: OsString, + args: Vec, env: Option>, ) -> std::io::Result { let mut child = Command::new(program)