diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index ff392a127d99..70a8ba16967a 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -46,6 +46,10 @@ fn build_permissions_update_item( .model_messages .as_ref() .and_then(|messages| messages.approvals.as_ref()), + next.model_info + .model_messages + .as_ref() + .and_then(|messages| messages.permissions.as_ref()), ), exec_policy, #[allow(deprecated)] diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 09b71139bb2a..b1bb26b94768 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -1414,6 +1414,7 @@ mod tests { auto_review: Some(AutoReviewMessages { policy: Some("Use the catalog Guardian policy.".to_string()), }), + permissions: None, }; let guardian_config = build_guardian_review_session_config( @@ -1441,6 +1442,7 @@ mod tests { auto_review: Some(AutoReviewMessages { policy: Some(String::new()), }), + permissions: None, }; let guardian_config = build_guardian_review_session_config( diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 99bb18ece200..8f7505523bfd 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3233,6 +3233,11 @@ impl Session { .model_messages .as_ref() .and_then(|messages| messages.approvals.as_ref()), + turn_context + .model_info + .model_messages + .as_ref() + .and_then(|messages| messages.permissions.as_ref()), ), self.services.exec_policy.current().as_ref(), #[allow(deprecated)] diff --git a/codex-rs/core/tests/suite/catalog_permission_messages.rs b/codex-rs/core/tests/suite/catalog_permission_messages.rs new file mode 100644 index 000000000000..3856bae8bb17 --- /dev/null +++ b/codex-rs/core/tests/suite/catalog_permission_messages.rs @@ -0,0 +1,115 @@ +use anyhow::Result; +use codex_core::config::Constrained; +use codex_login::CodexAuth; +use codex_models_manager::manager::RefreshStrategy; +use codex_models_manager::model_info::model_info_from_slug; +use codex_protocol::models::PermissionProfile; +use codex_protocol::openai_models::ModelMessages; +use codex_protocol::openai_models::ModelsResponse; +use codex_protocol::openai_models::PermissionMessages; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::protocol::ThreadSettingsOverrides; +use codex_protocol::user_input::UserInput; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_models_once; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::sse; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; +use wiremock::MockServer; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn catalog_permission_message_loaded_from_remote_models_is_sent() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = MockServer::start().await; + let model_slug = "remote-catalog-permissions-model"; + let mut model = model_info_from_slug(model_slug); + model.model_messages = Some(ModelMessages { + instructions_template: None, + instructions_variables: None, + approvals: None, + auto_review: None, + permissions: Some(PermissionMessages { + danger_full_access: None, + workspace_write: None, + read_only: Some("remote catalog permissions: {{ network_access }}".to_string()), + }), + }); + let models_mock = mount_models_once( + &server, + ModelsResponse { + models: vec![model], + }, + ) + .await; + let response_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let mut builder = test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(|config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never); + config + .permissions + .set_permission_profile(PermissionProfile::read_only()) + .expect("read-only permission profile should be allowed"); + }); + let test = builder.build_with_auto_env(&server).await?; + test.thread_manager + .get_models_manager() + .list_models( + RefreshStrategy::OnlineIfUncached, + codex_core::test_support::default_http_client_factory(), + ) + .await; + + core_test_support::submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + model: Some(model_slug.to_string()), + ..Default::default() + }, + ) + .await?; + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert_eq!(models_mock.single_request_path(), "/v1/models"); + let permissions = response_mock + .single_request() + .message_input_texts("developer") + .into_iter() + .filter(|text| text.contains("")) + .map(|text| text.replace("\r\n", "\n")) + .collect::>(); + assert_eq!( + permissions, + vec![ + "\nremote catalog permissions: restricted\nApproval policy is currently never. Do not provide the `sandbox_permissions` for any reason, commands will be rejected.\n" + .to_string() + ] + ); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index a6cfc85d7b8c..f0b3fecb72d1 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -38,6 +38,7 @@ mod apply_patch_cli; #[cfg(not(target_os = "windows"))] mod approvals; mod auto_review; +mod catalog_permission_messages; mod cli_stream; mod client; mod client_websockets; diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index 6503c0407f38..4a9e9ea3a28f 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -11,6 +11,7 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ApprovalMessages; use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ModelsResponse; +use codex_protocol::openai_models::PermissionMessages; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; @@ -52,6 +53,22 @@ fn model_with_approval_messages( on_request_auto_review: Some(on_request_auto_review.to_string()), }), auto_review: None, + permissions: None, + }); + model +} + +fn model_with_permission_messages( + slug: &str, + permissions: PermissionMessages, +) -> codex_protocol::openai_models::ModelInfo { + let mut model = model_info_from_slug(slug); + model.model_messages = Some(ModelMessages { + instructions_template: None, + instructions_variables: None, + approvals: None, + auto_review: None, + permissions: Some(permissions), }); model } @@ -162,6 +179,128 @@ async fn model_change_appends_new_catalog_approval_message() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn catalog_permission_message_is_sent_initially_and_after_model_change() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let _req1 = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let req2 = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]), + ) + .await; + let first_slug = "catalog-permissions-model-a"; + let second_slug = "catalog-permissions-model-b"; + let first = model_with_permission_messages( + first_slug, + PermissionMessages { + danger_full_access: None, + workspace_write: None, + read_only: Some("model A permissions: {{ network_access }}".to_string()), + }, + ); + let second = model_with_permission_messages( + second_slug, + PermissionMessages { + danger_full_access: None, + workspace_write: None, + read_only: Some("model B permissions: {{ network_access }}".to_string()), + }, + ); + let mut builder = test_codex() + .with_model(first_slug) + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never); + config + .permissions + .set_permission_profile(PermissionProfile::read_only()) + .expect("read-only permission profile should be allowed"); + config.model_catalog = Some(ModelsResponse { + models: vec![first, second], + }); + }); + let test = builder.build(&server).await?; + submit_text_turn(&test, "first").await?; + + core_test_support::submit_thread_settings( + &test.codex, + codex_protocol::protocol::ThreadSettingsOverrides { + model: Some(second_slug.to_string()), + ..Default::default() + }, + ) + .await?; + submit_text_turn(&test, "second").await?; + + let permissions = permissions_texts(&req2.single_request()); + assert_eq!(permissions.len(), 2); + assert!(permissions[0].contains("model A permissions: restricted")); + assert!( + permissions + .last() + .is_some_and(|text| text.contains("model B permissions: restricted")) + ); + assert!( + permissions + .last() + .is_some_and(|text| text.contains("Approval policy is currently never")) + ); + assert!( + !permissions + .iter() + .any(|text| text.contains("`sandbox_mode`")) + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn empty_catalog_permission_message_preserves_approval_instructions() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let req = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let model_slug = "empty-catalog-permissions-model"; + let model = model_with_permission_messages( + model_slug, + PermissionMessages { + danger_full_access: None, + workspace_write: None, + read_only: Some(String::new()), + }, + ); + let mut builder = test_codex() + .with_model(model_slug) + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never); + config + .permissions + .set_permission_profile(PermissionProfile::read_only()) + .expect("read-only permission profile should be allowed"); + config.model_catalog = Some(ModelsResponse { + models: vec![model], + }); + }); + let test = builder.build(&server).await?; + + submit_text_turn(&test, "hello").await?; + + let permissions = permissions_texts(&req.single_request()); + assert_eq!(permissions.len(), 1); + assert!(permissions[0].contains("Approval policy is currently never")); + assert!(!permissions[0].contains("Filesystem sandboxing defines")); + assert!(!permissions[0].contains("`sandbox_mode`")); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn permissions_message_sent_once_on_start() -> Result<()> { skip_if_no_network!(Ok(())); @@ -692,7 +831,11 @@ async fn permissions_message_includes_writable_roots() -> Result<()> { let expected = PermissionsInstructions::from_permission_profile( &permission_profile, AskForApproval::OnRequest, - ApprovalPromptContext::new(test.config.approvals_reviewer, /*messages*/ None), + ApprovalPromptContext::new( + test.config.approvals_reviewer, + /*messages*/ None, + /*permission_messages*/ None, + ), &exec_policy, test.config.cwd.as_path(), /*exec_permission_approvals_enabled*/ false, diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 08272caa104d..aa71da4e80e7 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -641,6 +641,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow }), approvals: None, auto_review: None, + permissions: None, }), include_skills_usage_instructions: false, supports_reasoning_summary_parameter: true, @@ -759,6 +760,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - }), approvals: None, auto_review: None, + permissions: None, }), include_skills_usage_instructions: false, supports_reasoning_summary_parameter: true, diff --git a/codex-rs/models-manager/src/model_info.rs b/codex-rs/models-manager/src/model_info.rs index e7b9ca6241c2..f8c2ee3ecc0d 100644 --- a/codex-rs/models-manager/src/model_info.rs +++ b/codex-rs/models-manager/src/model_info.rs @@ -112,7 +112,10 @@ fn clear_instruction_messages(model: &mut ModelInfo) { if let Some(model_messages) = &mut model.model_messages { model_messages.instructions_template = None; model_messages.instructions_variables = None; - if model_messages.approvals.is_none() && model_messages.auto_review.is_none() { + if model_messages.approvals.is_none() + && model_messages.auto_review.is_none() + && model_messages.permissions.is_none() + { model.model_messages = None; } } @@ -177,6 +180,7 @@ fn local_personality_messages_for_slug(slug: &str) -> Option { }), approvals: None, auto_review: None, + permissions: None, }), _ => None, } diff --git a/codex-rs/models-manager/src/model_info_tests.rs b/codex-rs/models-manager/src/model_info_tests.rs index d265695fa132..ef410a1e5821 100644 --- a/codex-rs/models-manager/src/model_info_tests.rs +++ b/codex-rs/models-manager/src/model_info_tests.rs @@ -3,6 +3,7 @@ use crate::ModelsManagerConfig; use codex_protocol::config_types::Personality; use codex_protocol::openai_models::ApprovalMessages; use codex_protocol::openai_models::AutoReviewMessages; +use codex_protocol::openai_models::PermissionMessages; use pretty_assertions::assert_eq; fn config_with_personality(personality: Option) -> ModelsManagerConfig { @@ -29,6 +30,7 @@ fn base_instruction_override_preserves_catalog_approval_messages() { }), approvals: Some(approvals.clone()), auto_review: None, + permissions: None, }); let config = ModelsManagerConfig { base_instructions: Some("override".to_string()), @@ -44,6 +46,7 @@ fn base_instruction_override_preserves_catalog_approval_messages() { instructions_variables: None, approvals: Some(approvals), auto_review: None, + permissions: None, }) ); } @@ -60,6 +63,7 @@ fn disabled_personality_preserves_catalog_approval_messages() { instructions_variables: None, approvals: Some(approvals.clone()), auto_review: None, + permissions: None, }); let config = ModelsManagerConfig { personality_enabled: false, @@ -75,6 +79,7 @@ fn disabled_personality_preserves_catalog_approval_messages() { instructions_variables: None, approvals: Some(approvals), auto_review: None, + permissions: None, }) ); } @@ -90,6 +95,7 @@ fn base_instruction_override_preserves_catalog_auto_review_messages() { instructions_variables: None, approvals: None, auto_review: Some(auto_review.clone()), + permissions: None, }); let config = ModelsManagerConfig { base_instructions: Some("override".to_string()), @@ -105,6 +111,41 @@ fn base_instruction_override_preserves_catalog_auto_review_messages() { instructions_variables: None, approvals: None, auto_review: Some(auto_review), + permissions: None, + }) + ); +} + +#[test] +fn base_instruction_override_preserves_catalog_permission_messages() { + let mut model = model_info_from_slug("unknown-model"); + let permissions = PermissionMessages { + danger_full_access: Some("danger".to_string()), + workspace_write: Some(String::new()), + read_only: None, + }; + model.model_messages = Some(ModelMessages { + instructions_template: Some("template".to_string()), + instructions_variables: None, + approvals: None, + auto_review: None, + permissions: Some(permissions.clone()), + }); + let config = ModelsManagerConfig { + base_instructions: Some("override".to_string()), + ..Default::default() + }; + + let updated = with_config_overrides(model, &config); + + assert_eq!( + updated.model_messages, + Some(ModelMessages { + instructions_template: None, + instructions_variables: None, + approvals: None, + auto_review: None, + permissions: Some(permissions), }) ); } @@ -140,6 +181,7 @@ fn personality_none_strips_catalog_instruction_sources_through_the_next_h1() { instructions_variables: None, approvals: None, auto_review: None, + permissions: None, }); let updated = with_config_overrides(model, &config); diff --git a/codex-rs/prompts/src/permissions_instructions.rs b/codex-rs/prompts/src/permissions_instructions.rs index 1c19d008cf33..a438812c22a4 100644 --- a/codex-rs/prompts/src/permissions_instructions.rs +++ b/codex-rs/prompts/src/permissions_instructions.rs @@ -5,6 +5,7 @@ use codex_protocol::config_types::SandboxMode; use codex_protocol::models::PermissionProfile; use codex_protocol::models::format_allow_prefixes; use codex_protocol::openai_models::ApprovalMessages; +use codex_protocol::openai_models::PermissionMessages; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; @@ -24,6 +25,7 @@ const APPROVAL_POLICY_ON_REQUEST_RULE: &str = const APPROVAL_POLICY_ON_REQUEST_RULE_REQUEST_PERMISSION: &str = include_str!("../templates/permissions/approval_policy/on_request_rule_request_permission.md"); const AUTO_REVIEW_APPROVAL_SUFFIX: &str = "`approvals_reviewer` is `auto_review`: Sandbox escalations with require_escalated will be reviewed for compliance with the policy. If a rejection happens, you should proceed only with a materially safer alternative, or inform the user of the risk and send a final message to ask for approval."; +const NETWORK_ACCESS_PLACEHOLDER: &str = "{{ network_access }}"; const SANDBOX_MODE_DANGER_FULL_ACCESS: &str = include_str!("../templates/permissions/sandbox_mode/danger_full_access.md"); @@ -49,6 +51,7 @@ struct PermissionsPromptConfig<'a> { approval_policy: AskForApproval, approvals_reviewer: ApprovalsReviewer, approval_messages: Option<&'a ApprovalMessages>, + permission_messages: Option<&'a PermissionMessages>, exec_policy: &'a Policy, exec_permission_approvals_enabled: bool, request_permissions_tool_enabled: bool, @@ -64,11 +67,20 @@ pub struct PermissionsInstructions { pub struct ApprovalPromptContext<'a> { reviewer: ApprovalsReviewer, messages: Option<&'a ApprovalMessages>, + permission_messages: Option<&'a PermissionMessages>, } impl<'a> ApprovalPromptContext<'a> { - pub fn new(reviewer: ApprovalsReviewer, messages: Option<&'a ApprovalMessages>) -> Self { - Self { reviewer, messages } + pub fn new( + reviewer: ApprovalsReviewer, + messages: Option<&'a ApprovalMessages>, + permission_messages: Option<&'a PermissionMessages>, + ) -> Self { + Self { + reviewer, + messages, + permission_messages, + } } } @@ -94,6 +106,7 @@ impl PermissionsInstructions { approval_policy, approvals_reviewer: approval_context.reviewer, approval_messages: approval_context.messages, + permission_messages: approval_context.permission_messages, exec_policy, exec_permission_approvals_enabled, request_permissions_tool_enabled, @@ -131,7 +144,10 @@ impl PermissionsInstructions { denied_reads: Option, ) -> Self { let mut text = String::new(); - append_section(&mut text, &sandbox_text(sandbox_mode, network_access)); + let sandbox = sandbox_text(sandbox_mode, network_access, config.permission_messages); + if !sandbox.is_empty() { + append_section(&mut text, &sandbox); + } append_section( &mut text, &approval_text( @@ -272,7 +288,24 @@ fn approval_text( } } -fn sandbox_text(mode: SandboxMode, network_access: NetworkAccess) -> String { +fn sandbox_text( + mode: SandboxMode, + network_access: NetworkAccess, + permission_messages: Option<&PermissionMessages>, +) -> String { + let selected = permission_messages.and_then(|messages| match mode { + SandboxMode::DangerFullAccess => messages.danger_full_access.as_deref(), + SandboxMode::WorkspaceWrite => messages.workspace_write.as_deref(), + SandboxMode::ReadOnly => messages.read_only.as_deref(), + }); + if let Some(selected) = selected { + if selected.is_empty() { + return String::new(); + } + let network_access = network_access.to_string(); + return selected.replace(NETWORK_ACCESS_PLACEHOLDER, network_access.as_str()); + } + let template = match mode { SandboxMode::DangerFullAccess => &*SANDBOX_MODE_DANGER_FULL_ACCESS_TEMPLATE, SandboxMode::WorkspaceWrite => &*SANDBOX_MODE_WORKSPACE_WRITE_TEMPLATE, diff --git a/codex-rs/prompts/src/permissions_instructions_tests.rs b/codex-rs/prompts/src/permissions_instructions_tests.rs index d8918b123527..5a82cd5da908 100644 --- a/codex-rs/prompts/src/permissions_instructions_tests.rs +++ b/codex-rs/prompts/src/permissions_instructions_tests.rs @@ -13,21 +13,148 @@ use std::path::PathBuf; #[test] fn renders_sandbox_mode_text() { assert_eq!( - sandbox_text(SandboxMode::WorkspaceWrite, NetworkAccess::Restricted), + sandbox_text( + SandboxMode::WorkspaceWrite, + NetworkAccess::Restricted, + /*permission_messages*/ None, + ), "Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval. Network access is restricted." ); assert_eq!( - sandbox_text(SandboxMode::ReadOnly, NetworkAccess::Restricted), + sandbox_text( + SandboxMode::ReadOnly, + NetworkAccess::Restricted, + /*permission_messages*/ None, + ), "Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The sandbox only permits reading files. Network access is restricted." ); assert_eq!( - sandbox_text(SandboxMode::DangerFullAccess, NetworkAccess::Enabled), + sandbox_text( + SandboxMode::DangerFullAccess, + NetworkAccess::Enabled, + /*permission_messages*/ None, + ), "Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `danger-full-access`: No filesystem sandboxing - all commands are permitted. Network access is enabled." ); } +#[test] +fn catalog_permission_messages_select_sandbox_mode_and_render_network_access() { + let messages = PermissionMessages { + danger_full_access: Some("catalog danger".to_string()), + workspace_write: Some("catalog workspace {{ network_access }}".to_string()), + read_only: Some("catalog read only {{ network_access }}".to_string()), + }; + + for (mode, expected) in [ + (SandboxMode::DangerFullAccess, "catalog danger"), + (SandboxMode::WorkspaceWrite, "catalog workspace enabled"), + (SandboxMode::ReadOnly, "catalog read only enabled"), + ] { + assert_eq!( + sandbox_text(mode, NetworkAccess::Enabled, Some(&messages)), + expected + ); + } +} + +#[test] +fn missing_catalog_permission_message_uses_legacy_sandbox_text() { + let legacy = sandbox_text( + SandboxMode::WorkspaceWrite, + NetworkAccess::Restricted, + /*permission_messages*/ None, + ); + let messages = PermissionMessages { + danger_full_access: None, + workspace_write: None, + read_only: Some("unused".to_string()), + }; + + assert_eq!( + sandbox_text( + SandboxMode::WorkspaceWrite, + NetworkAccess::Restricted, + Some(&messages), + ), + legacy + ); +} + +#[test] +fn invalid_catalog_permission_message_is_preserved_verbatim() { + for workspace_write in ["{{ unterminated", "{{ unsupported }}"] { + let messages = PermissionMessages { + danger_full_access: None, + workspace_write: Some(workspace_write.to_string()), + read_only: None, + }; + assert_eq!( + sandbox_text( + SandboxMode::WorkspaceWrite, + NetworkAccess::Restricted, + Some(&messages), + ), + workspace_write + ); + } +} + +#[test] +fn catalog_permission_message_renders_network_access_and_preserves_other_placeholders() { + let source = "network={{ network_access }} compact={{network_access}} other={{ other }}"; + let messages = PermissionMessages { + danger_full_access: None, + workspace_write: Some(source.to_string()), + read_only: None, + }; + + assert_eq!( + sandbox_text( + SandboxMode::WorkspaceWrite, + NetworkAccess::Restricted, + Some(&messages), + ), + "network=restricted compact={{network_access}} other={{ other }}" + ); +} + +#[test] +fn empty_catalog_permission_message_preserves_non_sandbox_sections() { + let messages = PermissionMessages { + danger_full_access: None, + workspace_write: Some(String::new()), + read_only: None, + }; + let writable_root = + AbsolutePathBuf::from_absolute_path(test_path_buf("/tmp/repo")).expect("absolute path"); + let instructions = PermissionsInstructions::from_permissions_with_network( + SandboxMode::WorkspaceWrite, + NetworkAccess::Restricted, + PermissionsPromptConfig { + approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, + approval_messages: None, + permission_messages: Some(&messages), + exec_policy: &Policy::empty(), + exec_permission_approvals_enabled: false, + request_permissions_tool_enabled: false, + }, + Some(vec![WritableRoot { + root: writable_root.clone(), + read_only_subpaths: Vec::new(), + protected_metadata_names: Vec::new(), + }]), + ); + let text = instructions.body(); + + assert!(!text.contains("Filesystem sandboxing defines")); + assert!(text.contains("Approval policy is currently never")); + assert!(text.contains(writable_root.to_string_lossy().as_ref())); +} + #[test] fn builds_permissions_with_network_access_override() { let instructions = PermissionsInstructions::from_permissions_with_network( @@ -37,6 +164,7 @@ fn builds_permissions_with_network_access_override() { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::User, approval_messages: None, + permission_messages: None, exec_policy: &Policy::empty(), exec_permission_approvals_enabled: false, request_permissions_tool_enabled: false, @@ -73,7 +201,11 @@ fn builds_permissions_from_profile() { let instructions = PermissionsInstructions::from_permission_profile( &permission_profile, AskForApproval::UnlessTrusted, - ApprovalPromptContext::new(ApprovalsReviewer::User, /*messages*/ None), + ApprovalPromptContext::new( + ApprovalsReviewer::User, + /*messages*/ None, + /*permission_messages*/ None, + ), &Policy::empty(), &cwd, /*exec_permission_approvals_enabled*/ false, @@ -118,7 +250,11 @@ fn builds_permissions_from_profile_with_denied_reads() { let instructions = PermissionsInstructions::from_permission_profile( &permission_profile, AskForApproval::OnRequest, - ApprovalPromptContext::new(ApprovalsReviewer::AutoReview, /*messages*/ None), + ApprovalPromptContext::new( + ApprovalsReviewer::AutoReview, + /*messages*/ None, + /*permission_messages*/ None, + ), &Policy::empty(), &cwd, /*exec_permission_approvals_enabled*/ false, @@ -144,6 +280,7 @@ fn includes_request_rule_instructions_for_on_request() { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::User, approval_messages: None, + permission_messages: None, exec_policy: &exec_policy, exec_permission_approvals_enabled: false, request_permissions_tool_enabled: false, @@ -166,6 +303,7 @@ fn includes_request_permissions_tool_instructions_for_unless_trusted_when_enable approval_policy: AskForApproval::UnlessTrusted, approvals_reviewer: ApprovalsReviewer::User, approval_messages: None, + permission_messages: None, exec_policy: &Policy::empty(), exec_permission_approvals_enabled: false, request_permissions_tool_enabled: true, @@ -187,6 +325,7 @@ fn includes_request_permission_rule_instructions_for_on_request_when_enabled() { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::User, approval_messages: None, + permission_messages: None, exec_policy: &Policy::empty(), exec_permission_approvals_enabled: true, request_permissions_tool_enabled: false, @@ -208,6 +347,7 @@ fn includes_request_permissions_tool_instructions_for_on_request_when_tool_is_en approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::User, approval_messages: None, + permission_messages: None, exec_policy: &Policy::empty(), exec_permission_approvals_enabled: false, request_permissions_tool_enabled: true, @@ -229,6 +369,7 @@ fn on_request_includes_tool_guidance_alongside_inline_permission_guidance_when_b approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::User, approval_messages: None, + permission_messages: None, exec_policy: &Policy::empty(), exec_permission_approvals_enabled: true, request_permissions_tool_enabled: true, @@ -289,6 +430,7 @@ fn empty_catalog_approval_message_suppresses_legacy_approval_section() { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::User, approval_messages: Some(&messages), + permission_messages: None, exec_policy: &exec_policy, exec_permission_approvals_enabled: true, request_permissions_tool_enabled: true, diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index bc0e05a7f377..921a1c458be2 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -506,6 +506,7 @@ pub struct ModelMessages { pub instructions_variables: Option, pub approvals: Option, pub auto_review: Option, + pub permissions: Option, } #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] @@ -519,6 +520,13 @@ pub struct AutoReviewMessages { pub policy: Option, } +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] +pub struct PermissionMessages { + pub danger_full_access: Option, + pub workspace_write: Option, + pub read_only: Option, +} + impl ModelMessages { fn has_personality_placeholder(&self) -> bool { self.instructions_template @@ -744,6 +752,7 @@ mod tests { .expect("model messages should deserialize"); assert_eq!(messages.approvals, None); + assert_eq!(messages.permissions, None); } #[test] @@ -768,6 +777,29 @@ mod tests { ); } + #[test] + fn permission_messages_preserve_missing_and_empty_values() { + let messages: ModelMessages = from_str( + r#"{ + "instructions_template": null, + "instructions_variables": null, + "permissions": { + "workspace_write": "" + } + }"#, + ) + .expect("permission messages should deserialize"); + + assert_eq!( + messages.permissions, + Some(PermissionMessages { + danger_full_access: None, + workspace_write: Some(String::new()), + read_only: None, + }) + ); + } + #[test] fn reasoning_effort_accepts_known_and_custom_values() { let custom = ReasoningEffort::Custom("future".to_string()); @@ -841,6 +873,7 @@ mod tests { instructions_variables: Some(personality_variables()), approvals: None, auto_review: None, + permissions: None, })); let instructions = model.get_model_instructions(Some(Personality::Friendly)); @@ -859,6 +892,7 @@ mod tests { }), approvals: None, auto_review: None, + permissions: None, })); assert_eq!( model.get_model_instructions(Some(Personality::Friendly)), @@ -886,6 +920,7 @@ mod tests { }), approvals: None, auto_review: None, + permissions: None, })); assert_eq!( model_no_personality.get_model_instructions(Some(Personality::Friendly)), @@ -916,6 +951,7 @@ mod tests { }), approvals: None, auto_review: None, + permissions: None, })); let instructions = model.get_model_instructions(Some(Personality::Friendly));