Commit Graph

2 Commits

  • fix: add integration tests for codex-exec-mcp-server with execpolicy (#7617)
    This PR introduces integration tests that run
    [codex-shell-tool-mcp](https://www.npmjs.com/package/@openai/codex-shell-tool-mcp)
    as a user would. Note that this requires running our fork of Bash, so we
    introduce a [DotSlash](https://dotslash-cli.com/) file for `bash` so
    that we can run the integration tests on multiple platforms without
    having to check the binaries into the repository. (As noted in the
    DotSlash file, it is slightly more heavyweight than necessary, which may
    be worth addressing as disk space in CI is limited:
    https://github.com/openai/codex/pull/7678.)
    
    To start, this PR adds two tests:
    
    - `list_tools()` makes the `list_tools` request to the MCP server and
    verifies we get the expected response
    - `accept_elicitation_for_prompt_rule()` defines a `prefix_rule()` with
    `decision="prompt"` and verifies the elicitation flow works as expected
    
    Though the `accept_elicitation_for_prompt_rule()` test **only works on
    Linux**, as this PR reveals that there are currently issues when running
    the Bash fork in a read-only sandbox on Linux. This will have to be
    fixed in a follow-up PR.
    
    Incidentally, getting this test run to correctly on macOS also requires
    a recent fix we made to `brew` that hasn't hit a mainline release yet,
    so getting CI green in this PR required
    https://github.com/openai/codex/pull/7680.
  • chore: refactor exec-server to prepare it for standalone MCP use (#6944)
    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:
    
    <img width="1589" height="1335" alt="image"
    src="https://github.com/user-attachments/assets/9db6aea8-7fbc-4675-8b1f-ec446685d6c4"
    />
    
    then I get prompted with the following elicitation, as expected:
    
    <img width="1589" height="1335" alt="image"
    src="https://github.com/user-attachments/assets/21b68fe0-494d-4562-9bad-0ddc55fc846d"
    />
    
    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;
    +    }
    +
    ```