Skip to content

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Nov 19, 2025

This PR reorganizes things slightly so that:

  • Instead of a single multitool executable, codex-exec-server, we now have two executables:
    • codex-exec-mcp-server to launch the MCP server
    • codex-execve-wrapper is the execve(2) wrapper to use with the BASH_EXEC_WRAPPER environment variable
  • BASH_EXEC_WRAPPER must be a single executable: it cannot be a command string composed of an executable with args (i.e., it no longer adds the escalate subcommand, as before)
  • codex-exec-mcp-server takes --bash and --execve as options. Though if --execve is not specified, the MCP server will check the directory containing std::env::current_exe() and attempt to use the file named codex-execve-wrapper within it. In development, this works out since these executables are side-by-side in the target/debug folder.

With respect to testing, this also fixes an important bug in dummy_exec_policy(), as I was using ends_with() as if it applied to a String, but in this case, it is used with a &Path, so the semantics are slightly different.

Putting this all together, I was able to test this by running the following:

~/code/codex/codex-rs$ npx @modelcontextprotocol/inspector \
    ./target/debug/codex-exec-mcp-server --bash ~/code/bash/bash

If I try to run git status in /Users/mbolin/code/codex via the shell tool from the MCP server:

image

then I get prompted with the following elicitation, as expected:

image

Though a current limitation is that the shell tool defaults to a timeout of 10s, which means I only have 10s to respond to the elicitation. Ideally, the time spent waiting for a response from a human should not count against the timeout for the command execution. I will address this in a subsequent PR.


Note ~/code/bash/bash was created by doing:

cd ~/code
git clone https://github.com/bminor/bash
cd bash
git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b
<apply the patch below>
./configure
make

The patch:

diff --git a/execute_cmd.c b/execute_cmd.c
index 070f5119..d20ad2b9 100644
--- a/execute_cmd.c
+++ b/execute_cmd.c
@@ -6129,6 +6129,19 @@ shell_execve (char *command, char **args, char **env)
   char sample[HASH_BANG_BUFSIZ];
   size_t larray;

+  char* exec_wrapper = getenv("BASH_EXEC_WRAPPER");
+  if (exec_wrapper && *exec_wrapper && !whitespace (*exec_wrapper))
+    {
+      char *orig_command = command;
+
+      larray = strvec_len (args);
+
+      memmove (args + 2, args, (++larray) * sizeof (char *));
+      args[0] = exec_wrapper;
+      args[1] = orig_command;
+      command = exec_wrapper;
+    }
+

@bolinfest bolinfest marked this pull request as ready for review November 19, 2025 23:38
Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 142 to 144
let ExecveWrapperCli { file, argv } = ExecveWrapperCli::parse();
escalate_client::run(file, argv).await?;
Ok(())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Propagate exec wrapper exit codes

The new main_execve_wrapper drops the exit code returned by escalate_client::run and always returns Ok(()), so any command that is escalated or denied now exits with status 0 instead of the real command/denial code. When the exec policy escalates or blocks a command, the shell will treat the operation as a success, changing control flow and hiding policy enforcement. The wrapper needs to exit with the code from escalate_client::run (as the prior Escalate subcommand did) to preserve the caller’s semantics.

Useful? React with 👍 / 👎.

@bolinfest bolinfest merged commit 13d378f into main Nov 20, 2025
86 of 92 checks passed
@bolinfest bolinfest deleted the pr6944 branch November 20, 2025 00:38
@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants