clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-28 00:25:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Ensure shell command skills trigger approval (#12697)" (#12721)

Browse Source 
This reverts commit daf0f03ac8.

# External (non-OpenAI) Pull Request Requirements

Before opening this Pull Request, please read the dedicated
"Contributing" markdown file or your PR may be closed:
https://github.com/openai/codex/blob/main/docs/contributing.md

If your PR conforms to our contribution guidelines, replace this text
with a detailed and high quality description of your changes.

Include a link to a bug report or enhancement request.
This commit is contained in:
Celia Chen
2026-02-25 14:49:53 -08:00
committed by GitHub
parent f86087eaa8
commit b6d20748e0
10 changed files with 83 additions and 556 deletions
Download Patch File Download Diff File Expand all files Collapse all files

298
codex-rs/core/tests/suite/skill_approval.rs
View File

@@ -1,23 +1,17 @@
#![allow(clippy::expect_used, clippy::unwrap_used)]
#![allow(clippy::unwrap_used)]
#![cfg(unix)]
use anyhow::Result;
use codex_core::features::Feature;
#[cfg(unix)]
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::models::FileSystemPermissions;
#[cfg(unix)]
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::skill_approval::SkillApprovalResponse;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
use core_test_support::responses::ev_response_created;
use codex_protocol::user_input::UserInput;
use core_test_support::responses::mount_function_call_agent_response;
use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::test_codex::TestCodex;
@@ -30,26 +24,6 @@ use std::fs;
use std::path::Path;
use std::path::PathBuf;
fn write_skill_with_script(home: &Path, name: &str, script_body: &str) -> Result<PathBuf> {
let skill_dir = home.join("skills").join(name);
let scripts_dir = skill_dir.join("scripts");
fs::create_dir_all(&scripts_dir)?;
fs::write(
skill_dir.join("SKILL.md"),
format!(
r#"---
name: {name}
description: {name} skill
---
"#
),
)?;
let script_path = scripts_dir.join("run.py");
fs::write(&script_path, script_body)?;
Ok(script_path)
}
#[cfg(unix)]
fn write_skill_metadata(home: &Path, name: &str, contents: &str) -> Result<()> {
let metadata_dir = home.join("skills").join(name).join("agents");
fs::create_dir_all(&metadata_dir)?;
@@ -64,40 +38,15 @@ fn shell_command_arguments(command: &str) -> Result<String> {
}))?)
}
fn assistant_response(message: &str) -> String {
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", message),
ev_completed("resp-2"),
])
}
fn command_for_script(script_path: &Path) -> Result<String> {
let runner = if cfg!(windows) { "python" } else { "python3" };
let script_path = script_path.to_string_lossy().into_owned();
Ok(shlex::try_join([runner, script_path.as_str()])?)
}
async fn submit_turn(test: &TestCodex, prompt: &str) -> Result<()> {
submit_turn_with_policies(
test,
prompt,
AskForApproval::Never,
SandboxPolicy::DangerFullAccess,
)
.await
}
async fn submit_turn_with_policies(
test: &TestCodex,
prompt: &str,
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
) -> Result<()> {
let session_model = test.session_configured.model.clone();
test.codex
.submit(Op::UserTurn {
items: vec![codex_protocol::user_input::UserInput::Text {
items: vec![UserInput::Text {
text: prompt.to_string(),
text_elements: Vec::new(),
}],
@@ -105,9 +54,9 @@ async fn submit_turn_with_policies(
cwd: test.cwd_path().to_path_buf(),
approval_policy,
sandbox_policy,
model: session_model,
model: test.session_configured.model.clone(),
effort: None,
summary: codex_protocol::config_types::ReasoningSummary::Auto,
summary: ReasoningSummary::Auto,
collaboration_mode: None,
personality: None,
})
@@ -126,7 +75,6 @@ async fn wait_for_turn_complete_without_skill_approval(test: &TestCodex) {
.await;
}
#[cfg(unix)]
fn write_skill_with_shell_script(home: &Path, name: &str, script_name: &str) -> Result<PathBuf> {
use std::os::unix::fs::PermissionsExt;
@@ -158,7 +106,6 @@ echo 'zsh-fork-stderr' >&2
Ok(script_path)
}
#[cfg(unix)]
fn find_test_zsh_path() -> Result<Option<PathBuf>> {
use core_test_support::fetch_dotslash_file;
@@ -181,7 +128,6 @@ fn find_test_zsh_path() -> Result<Option<PathBuf>> {
}
}
#[cfg(unix)]
fn supports_exec_wrapper_intercept(zsh_path: &Path) -> bool {
let status = std::process::Command::new(zsh_path)
.arg("-fc")
@@ -194,236 +140,6 @@ fn supports_exec_wrapper_intercept(zsh_path: &Path) -> bool {
}
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn skill_approval_event_round_trip_for_shell_command_skill_script_exec() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let tool_call_id = "shell-skill-call";
let mut builder = test_codex()
.with_pre_build_hook(|home| {
write_skill_with_script(home, "demo", "print('shell skill approved')").unwrap();
})
.with_config(|config| {
config.features.enable(Feature::SkillApproval);
});
let test = builder.build(&server).await?;
let script_path = test.codex_home_path().join("skills/demo/scripts/run.py");
let command = command_for_script(&script_path)?;
let arguments = shell_command_arguments(&command)?;
let _mocks =
mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command")
.await;
submit_turn(&test, "run the shell skill").await?;
let request = wait_for_event_match(test.codex.as_ref(), |event| match event {
EventMsg::SkillRequestApproval(request) => Some(request.clone()),
_ => None,
})
.await;
assert_eq!(request.item_id, tool_call_id);
assert_eq!(request.skill_name, "demo");
test.codex
.submit(Op::SkillApproval {
id: request.item_id,
response: SkillApprovalResponse { approved: true },
})
.await?;
wait_for_event(test.codex.as_ref(), |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn skill_approval_not_emitted_without_skill_script_exec() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let tool_call_id = "non-skill-call";
let mut builder = test_codex().with_config(|config| {
config.features.enable(Feature::SkillApproval);
});
let test = builder.build(&server).await?;
let arguments = shell_command_arguments("echo no-skill")?;
let _mocks =
mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command")
.await;
submit_turn(&test, "run a plain command").await?;
wait_for_turn_complete_without_skill_approval(&test).await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn skill_approval_decline_blocks_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let tool_call_id = "decline-call";
let marker_name = "declined-marker.txt";
let mut builder = test_codex()
.with_pre_build_hook(move |home| {
let marker_path = home.join(marker_name);
let marker_path = marker_path.to_string_lossy();
let script_body = format!(
r#"from pathlib import Path
Path({marker_path:?}).write_text('ran')
print('ran')
"#
);
write_skill_with_script(home, "demo", &script_body).unwrap();
})
.with_config(|config| {
config.features.enable(Feature::SkillApproval);
});
let test = builder.build(&server).await?;
let script_path = test.codex_home_path().join("skills/demo/scripts/run.py");
let command = command_for_script(&script_path)?;
let arguments = shell_command_arguments(&command)?;
let mocks =
mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command")
.await;
submit_turn(&test, "run the skill").await?;
let request = wait_for_event_match(test.codex.as_ref(), |event| match event {
EventMsg::SkillRequestApproval(request) => Some(request.clone()),
_ => None,
})
.await;
assert_eq!(request.item_id, tool_call_id);
test.codex
.submit(Op::SkillApproval {
id: request.item_id,
response: SkillApprovalResponse { approved: false },
})
.await?;
wait_for_event(test.codex.as_ref(), |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let marker_path = test.codex_home_path().join(marker_name);
assert!(
!marker_path.exists(),
"declined skill approval should block script execution"
);
let call_output = mocks
.completion
.single_request()
.function_call_output(tool_call_id);
assert_eq!(
call_output["output"].as_str(),
Some("This script is part of the skill and the user declined the skill usage"),
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn skill_approval_cache_is_per_skill() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let first_call_id = "skill-a-1";
let second_call_id = "skill-a-2";
let third_call_id = "skill-b-1";
let mut builder = test_codex()
.with_pre_build_hook(|home| {
write_skill_with_script(home, "alpha", "print('alpha')").unwrap();
write_skill_with_script(home, "beta", "print('beta')").unwrap();
})
.with_config(|config| {
config.features.enable(Feature::SkillApproval);
});
let test = builder.build(&server).await?;
let alpha_command =
command_for_script(&test.codex_home_path().join("skills/alpha/scripts/run.py"))?;
let beta_command =
command_for_script(&test.codex_home_path().join("skills/beta/scripts/run.py"))?;
let first_alpha_arguments = shell_command_arguments(&alpha_command)?;
let second_alpha_arguments = shell_command_arguments(&alpha_command)?;
let beta_arguments = shell_command_arguments(&beta_command)?;
mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(first_call_id, "shell_command", &first_alpha_arguments),
ev_completed("resp-1"),
]),
assistant_response("alpha-1"),
sse(vec![
ev_response_created("resp-1"),
ev_function_call(second_call_id, "shell_command", &second_alpha_arguments),
ev_completed("resp-1"),
]),
assistant_response("alpha-2"),
sse(vec![
ev_response_created("resp-1"),
ev_function_call(third_call_id, "shell_command", &beta_arguments),
ev_completed("resp-1"),
]),
assistant_response("beta-1"),
],
)
.await;
submit_turn(&test, "run alpha").await?;
let first_request = wait_for_event_match(test.codex.as_ref(), |event| match event {
EventMsg::SkillRequestApproval(request) => Some(request.clone()),
_ => None,
})
.await;
assert_eq!(first_request.item_id, first_call_id);
assert_eq!(first_request.skill_name, "alpha");
test.codex
.submit(Op::SkillApproval {
id: first_request.item_id,
response: SkillApprovalResponse { approved: true },
})
.await?;
wait_for_event(test.codex.as_ref(), |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
submit_turn(&test, "run alpha again").await?;
wait_for_turn_complete_without_skill_approval(&test).await;
submit_turn(&test, "run beta").await?;
let third_request = wait_for_event_match(test.codex.as_ref(), |event| match event {
EventMsg::SkillRequestApproval(request) => Some(request.clone()),
_ => None,
})
.await;
assert_eq!(third_request.item_id, third_call_id);
assert_eq!(third_request.skill_name, "beta");
test.codex
.submit(Op::SkillApproval {
id: third_request.item_id,
response: SkillApprovalResponse { approved: true },
})
.await?;
wait_for_event(test.codex.as_ref(), |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn shell_zsh_fork_prompts_for_skill_script_execution() -> Result<()> {