mirror of
https://github.com/openai/codex.git
synced 2026-10-15 16:48:52 +00:00
Test same-cell permission grants and strict review in code mode (#48017)
## What changed Add app-server integration coverage for awaiting `request_permissions` and then running `exec_command` in the same code-mode cell: - Verify that turn- and session-scoped write grants allow the command to create a file, including grant propagation to Windows command approval requests. - Verify that `strictAutoReview` immediately triggers review for a command needing no additional access and returns the reviewer's denial rationale. GitOrigin-RevId: 02c958071db4e9ba667be9ba95fbb3a506ae1572
This commit is contained in:
Adam Perry @ OpenAI
committed by
copyberry
copyberry
parent
d0a64e91ae
commit
d7b07d4551
@@ -1,9 +1,14 @@
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use app_test_support::MockResponsesConfig;
|
||||
use app_test_support::TestAppServer;
|
||||
use app_test_support::create_final_assistant_message_sse_response;
|
||||
use app_test_support::create_request_permissions_sse_response;
|
||||
use app_test_support::write_models_cache_with_models;
|
||||
use codex_app_server_protocol::AdditionalPermissionProfile;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::CommandExecutionApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::PermissionGrantScope;
|
||||
use codex_app_server_protocol::PermissionsRequestApprovalResponse;
|
||||
@@ -11,10 +16,14 @@ use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::ServerRequestResolvedNotification;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::TurnCompletedNotification;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use codex_exec_server::ReadFileOptions;
|
||||
use codex_features::Feature;
|
||||
use codex_models_manager::model_info::model_info_from_slug;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope as CorePermissionGrantScope;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
@@ -22,12 +31,214 @@ use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::skip_if_wine_exec;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use test_case::test_case;
|
||||
use tokio::time::timeout;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
|
||||
/// An awaited permission approval applies to the next command in the same code-mode cell.
|
||||
/// Strict review must also apply immediately, even to a command needing no additional access.
|
||||
#[test_case(PermissionGrantScope::Turn, false; "turn")]
|
||||
#[test_case(PermissionGrantScope::Session, false; "session")]
|
||||
#[test_case(PermissionGrantScope::Turn, true; "strict_review")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||
async fn code_mode_uses_permissions_approved_in_the_same_cell(
|
||||
scope: PermissionGrantScope,
|
||||
strict_auto_review: bool,
|
||||
) -> Result<()> {
|
||||
skip_if_wine_exec!(
|
||||
Ok(()),
|
||||
"Wine does not emulate Windows restricted-token and ACL sandbox semantics"
|
||||
);
|
||||
let codex_home = tempfile::TempDir::new()?;
|
||||
let server = responses::start_mock_server().await;
|
||||
MockResponsesConfig::new(&server.uri())
|
||||
.with_model("gpt-5.4")
|
||||
.with_approval_policy("on-request")
|
||||
.with_sandbox_mode("read-only")
|
||||
.with_root_config(r#"approvals_reviewer = "user""#)
|
||||
.enable_feature(Feature::RequestPermissionsTool)
|
||||
.enable_feature(Feature::CodeModeOnly)
|
||||
.enable_feature(Feature::GuardianApproval)
|
||||
.write(codex_home.path())?;
|
||||
write_models_cache_with_models(codex_home.path(), vec![model_info_from_slug("gpt-5.4")])
|
||||
.await?;
|
||||
let mut app = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let cwd = app.auto_env()?.selection().cwd.clone();
|
||||
let fs = app.auto_env()?.environment().get_filesystem();
|
||||
let ThreadStartResponse { thread, .. } = app.start_thread(ThreadStartParams::default()).await?;
|
||||
|
||||
let command = if strict_auto_review {
|
||||
"echo same-cell-review"
|
||||
} else {
|
||||
"echo same-cell-grant > granted.txt"
|
||||
};
|
||||
let mut events = vec![responses::sse(vec![
|
||||
responses::ev_response_created("same-cell"),
|
||||
responses::ev_custom_tool_call(
|
||||
"same-cell",
|
||||
"exec",
|
||||
&format!(
|
||||
r#"// @exec: {{"yield_time_ms": 60000}}
|
||||
await tools.request_permissions({{permissions: {{file_system: {{write: ["."]}}}}}});
|
||||
text(await tools.exec_command({{cmd: "{command}"}}));"#,
|
||||
),
|
||||
),
|
||||
responses::ev_completed("same-cell"),
|
||||
])];
|
||||
if strict_auto_review {
|
||||
events.push(responses::sse(vec![
|
||||
responses::ev_response_created("guardian-review"),
|
||||
responses::ev_assistant_message(
|
||||
"guardian-review",
|
||||
r#"{"outcome":"deny","rationale":"same-cell command requires review"}"#,
|
||||
),
|
||||
responses::ev_completed("guardian-review"),
|
||||
]));
|
||||
}
|
||||
events.push(create_final_assistant_message_sse_response("done")?);
|
||||
let mock = responses::mount_sse_sequence(&server, events).await;
|
||||
let TurnStartResponse { turn } = app
|
||||
.request(|request_id| ClientRequest::TurnStart {
|
||||
request_id,
|
||||
params: TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "request permission, then run the command in the same cell".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
let request = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
app.read_stream_until_request_message(),
|
||||
)
|
||||
.await??;
|
||||
let ServerRequest::PermissionsRequestApproval { request_id, params } = request else {
|
||||
anyhow::bail!("expected permission approval, got {request:?}");
|
||||
};
|
||||
assert_eq!((¶ms.thread_id, ¶ms.turn_id), (&thread.id, &turn.id));
|
||||
app.send_response(
|
||||
request_id,
|
||||
json!({
|
||||
"permissions": params.permissions,
|
||||
"scope": scope,
|
||||
"strictAutoReview": strict_auto_review,
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
let mut command_approved = false;
|
||||
loop {
|
||||
match timeout(DEFAULT_READ_TIMEOUT, app.read_next_message()).await?? {
|
||||
JSONRPCMessage::Request(request)
|
||||
if cfg!(windows) && !strict_auto_review && !command_approved =>
|
||||
{
|
||||
// Without a Windows sandbox backend, command approval is still required.
|
||||
// Check the grant before approving so stale same-cell permissions still fail.
|
||||
let ServerRequest::CommandExecutionRequestApproval {
|
||||
request_id,
|
||||
params: command_params,
|
||||
} = ServerRequest::try_from(request)?
|
||||
else {
|
||||
anyhow::bail!("expected Windows command approval");
|
||||
};
|
||||
assert_eq!(
|
||||
(
|
||||
command_params.thread_id,
|
||||
command_params.turn_id,
|
||||
command_params.environment_id,
|
||||
command_params.cwd,
|
||||
command_params.additional_permissions,
|
||||
),
|
||||
(
|
||||
thread.id.clone(),
|
||||
turn.id.clone(),
|
||||
params.environment_id.clone(),
|
||||
Some(params.cwd.clone()),
|
||||
Some(AdditionalPermissionProfile {
|
||||
network: params.permissions.network.clone(),
|
||||
file_system: params.permissions.file_system.clone(),
|
||||
}),
|
||||
),
|
||||
);
|
||||
app.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(CommandExecutionRequestApprovalResponse {
|
||||
decision: CommandExecutionApprovalDecision::Accept,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
command_approved = true;
|
||||
}
|
||||
JSONRPCMessage::Request(request) => {
|
||||
anyhow::bail!(
|
||||
"the approved command must not ask for another approval: {request:?}"
|
||||
);
|
||||
}
|
||||
JSONRPCMessage::Notification(notification)
|
||||
if notification.method == "turn/completed" =>
|
||||
{
|
||||
let completed: TurnCompletedNotification =
|
||||
serde_json::from_value(notification.params.expect("turn/completed params"))?;
|
||||
assert_eq!(
|
||||
(
|
||||
completed.thread_id,
|
||||
completed.turn.id,
|
||||
completed.turn.status
|
||||
),
|
||||
(thread.id, turn.id, TurnStatus::Completed),
|
||||
);
|
||||
break;
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
let requests = mock.requests();
|
||||
let output = requests
|
||||
.last()
|
||||
.expect("model continuation")
|
||||
.custom_tool_call_output("same-cell");
|
||||
if strict_auto_review {
|
||||
assert_eq!(
|
||||
requests
|
||||
.iter()
|
||||
.filter(|request| {
|
||||
request.body_json()["client_metadata"]["x-openai-subagent"] == "guardian"
|
||||
})
|
||||
.count(),
|
||||
1,
|
||||
"strict review must apply to the next command in the same cell; output: {output}",
|
||||
);
|
||||
assert!(
|
||||
output
|
||||
.to_string()
|
||||
.contains("same-cell command requires review")
|
||||
);
|
||||
} else {
|
||||
let contents = fs
|
||||
.read_file(
|
||||
&cwd.join("granted.txt")?,
|
||||
ReadFileOptions::default(),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.with_context(|| {
|
||||
format!("the approved same-cell command must write the file; output: {output}")
|
||||
})?;
|
||||
assert_eq!(String::from_utf8(contents)?.trim(), "same-cell-grant");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||
async fn request_permissions_round_trip() -> Result<()> {
|
||||
let codex_home = tempfile::TempDir::new()?;
|
||||
|
||||
Reference in New Issue
Block a user