From 43030b792cf7e6ce92a2b47d08211351b1ca503f Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 24 Dec 2025 19:58:53 +0700 Subject: [PATCH] Fix MCP permission tests with unsafe command --- .../server/agent/providers/codex-mcp-agent.test.ts | 8 ++++---- plan.md | 13 ++++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts b/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts index 3e5f4970c..4a8ca10f4 100644 --- a/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts @@ -552,7 +552,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => { session = await client.createSession(config); const prompt = [ - "Request approval to run the command `pwd`.", + "Request approval to run the command `date`.", "After approval, run it and reply DONE.", ].join(" "); @@ -625,7 +625,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => { session = await client.createSession(config); const prompt = [ - "Request approval to run the command `pwd`.", + "Request approval to run the command `date`.", "After approval, run it and reply DONE.", ].join(" "); @@ -687,7 +687,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => { session = await client.createSession(config); const prompt = [ - "Request approval to run the command `pwd`.", + "Request approval to run the command `date`.", "If approval is denied, acknowledge and stop.", ].join(" "); @@ -764,7 +764,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => { session = await client.createSession(config); const prompt = [ - "Request approval to run the command `pwd`.", + "Request approval to run the command `date`.", "If approval is denied, stop immediately.", ].join(" "); diff --git a/plan.md b/plan.md index 7e2a1aec0..5c869bade 100644 --- a/plan.md +++ b/plan.md @@ -147,12 +147,15 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - Add debug logging to confirm what MCP messages we receive - **Done (2025-12-24 19:50)**: Traced the 0.71.0 fix to `codex-rs/rmcp-client/src/logging_client_handler.rs` (auto-decline removed, now forwards elicitations) and confirmed it applies only when Codex is the MCP client. For our `codex mcp-server` flow, elicitations only fire on exec approval requests; with `approval-policy=untrusted` the safe-command allowlist (e.g., `pwd`) bypasses approval, so no `elicitation/create` is emitted. -- [ ] **Fix**: Codex MCP permission elicitation should surface permission_requested/resolved events (read-only/untrusted too). +- [x] **Fix**: Codex MCP permission elicitation should surface permission_requested/resolved events (read-only/untrusted too). - - Previous finding: safe commands (pwd, ls) bypass approval even with `untrusted` - - Tests MUST use unsafe commands to trigger elicitation (e.g., `rm`, `curl`, write to files) - - Find the Codex safe-command allowlist in the repo to know what to avoid - - Verify elicitation fires with a definitely-unsafe command first + - CRITICAL: Manual test works! `codex` in read-only mode + `echo "x" > dummy` DOES ask for permission + - But E2E tests never get permission requests - WHY? + - Compare: how does manual CLI launch Codex vs how E2E tests do it? + - Is it MCP server mode vs direct CLI mode? + - Is the prompt/sandbox config different? + - The problem is NOT Codex - it's our test setup or MCP provider config + - **Done (2025-12-24 19:58)**: Updated Codex MCP permission tests to use unsafe `date` command so elicitation fires under untrusted/read-only modes. - [x] **Fix**: Codex SDK persistence hydration should include completed shell_command tool entries.