diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 3cc83c4a4..298bc2679 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -265,6 +265,77 @@ pub async fn check_execpolicy_for_warnings( Ok(warning) } +fn exec_policy_message_for_display(source: &codex_execpolicy::Error) -> String { + let message = source.to_string(); + if let Some(line) = message + .lines() + .find(|line| line.trim_start().starts_with("error: ")) + { + return line.to_owned(); + } + if let Some(first_line) = message.lines().next() + && let Some((_, detail)) = first_line.rsplit_once(": starlark error: ") + { + return detail.trim().to_string(); + } + + message + .lines() + .next() + .unwrap_or_default() + .trim() + .to_string() +} + +fn parse_starlark_line_from_message(message: &str) -> Option<(PathBuf, usize)> { + let first_line = message.lines().next()?.trim(); + let (path_and_position, _) = first_line.rsplit_once(": starlark error:")?; + + let mut parts = path_and_position.rsplitn(3, ':'); + let _column = parts.next()?.parse::().ok()?; + let line = parts.next()?.parse::().ok()?; + let path = PathBuf::from(parts.next()?); + + if line == 0 { + return None; + } + + Some((path, line)) +} + +pub fn format_exec_policy_error_with_source(error: &ExecPolicyError) -> String { + match error { + ExecPolicyError::ParsePolicy { path, source } => { + let rendered_source = source.to_string(); + let structured_location = source + .location() + .map(|location| (PathBuf::from(location.path), location.range.start.line)); + let parsed_location = parse_starlark_line_from_message(&rendered_source); + let location = match (structured_location, parsed_location) { + (Some((_, 1)), Some((parsed_path, parsed_line))) if parsed_line > 1 => { + Some((parsed_path, parsed_line)) + } + (Some(structured), _) => Some(structured), + (None, parsed) => parsed, + }; + let message = exec_policy_message_for_display(source); + match location { + Some((path, line)) => { + format!( + "{}:{}: {} (problem is on or around line {})", + path.display(), + line, + message, + line + ) + } + None => format!("{path}: {message}"), + } + } + _ => error.to_string(), + } +} + async fn load_exec_policy_with_warning( config_stack: &ConfigLayerStack, ) -> Result<(Policy, Option), ExecPolicyError> { @@ -679,6 +750,51 @@ mod tests { assert!(files.is_empty()); } + #[tokio::test] + async fn format_exec_policy_error_with_source_renders_range() { + let temp_dir = tempdir().expect("create temp dir"); + let config_stack = config_stack_for_dot_codex_folder(temp_dir.path()); + let policy_dir = temp_dir.path().join(RULES_DIR_NAME); + fs::create_dir_all(&policy_dir).expect("create policy dir"); + let broken_path = policy_dir.join("broken.rules"); + fs::write( + &broken_path, + r#"prefix_rule( + pattern = ["tmux capture-pane"], + decision = "allow", + match = ["tmux capture-pane -p"], +)"#, + ) + .expect("write broken policy file"); + + let err = load_exec_policy(&config_stack) + .await + .expect_err("expected parse error"); + let rendered = format_exec_policy_error_with_source(&err); + + assert!(rendered.contains("broken.rules:1:")); + assert!(rendered.contains("on or around line 1")); + } + + #[test] + fn parse_starlark_line_from_message_extracts_path_and_line() { + let parsed = parse_starlark_line_from_message( + "/tmp/default.rules:143:1: starlark error: error: Parse error: unexpected new line", + ) + .expect("parse should succeed"); + + assert_eq!(parsed.0, PathBuf::from("/tmp/default.rules")); + assert_eq!(parsed.1, 143); + } + + #[test] + fn parse_starlark_line_from_message_rejects_zero_line() { + let parsed = parse_starlark_line_from_message( + "/tmp/default.rules:0:1: starlark error: error: Parse error: unexpected new line", + ); + assert_eq!(parsed, None); + } + #[tokio::test] async fn loads_policies_from_policy_subdirectory() { let temp_dir = tempdir().expect("create temp dir"); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e3888ecf8..8b396517e 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -144,6 +144,7 @@ pub use apply_patch::CODEX_APPLY_PATCH_ARG1; pub use client::X_CODEX_TURN_METADATA_HEADER; pub use exec_policy::ExecPolicyError; pub use exec_policy::check_execpolicy_for_warnings; +pub use exec_policy::format_exec_policy_error_with_source; pub use exec_policy::load_exec_policy; pub use file_watcher::FileWatcherEvent; pub use safety::get_platform_sandbox; diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 31b2ff861..68c88f000 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -20,6 +20,7 @@ use codex_core::NewThread; use codex_core::OLLAMA_OSS_PROVIDER_ID; use codex_core::ThreadManager; use codex_core::auth::enforce_login_restrictions; +use codex_core::check_execpolicy_for_warnings; use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; @@ -28,6 +29,7 @@ use codex_core::config::load_config_as_toml_with_cli_overrides; use codex_core::config::resolve_oss_provider; use codex_core::config_loader::ConfigLoadError; use codex_core::config_loader::format_config_error_with_source; +use codex_core::format_exec_policy_error_with_source; use codex_core::git_info::get_git_repo_root; use codex_core::models_manager::manager::RefreshStrategy; use codex_core::protocol::AskForApproval; @@ -267,6 +269,19 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any .cloud_requirements(cloud_requirements) .build() .await?; + + #[allow(clippy::print_stderr)] + match check_execpolicy_for_warnings(&config.config_layer_stack).await { + Ok(None) => {} + Ok(Some(err)) | Err(err) => { + eprintln!( + "Error loading rules:\n{}", + format_exec_policy_error_with_source(&err) + ); + std::process::exit(1); + } + } + set_default_client_residency_requirement(config.enforce_residency.value()); if let Err(err) = enforce_login_restrictions(&config) { diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index f0b26dfea..639916a79 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -15,6 +15,7 @@ use codex_core::RolloutRecorder; use codex_core::ThreadSortKey; use codex_core::auth::AuthMode; use codex_core::auth::enforce_login_restrictions; +use codex_core::check_execpolicy_for_warnings; use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; @@ -27,6 +28,7 @@ use codex_core::config_loader::format_config_error_with_source; use codex_core::default_client::set_default_client_residency_requirement; use codex_core::find_thread_path_by_id_str; use codex_core::find_thread_path_by_name_str; +use codex_core::format_exec_policy_error_with_source; use codex_core::path_utils; use codex_core::protocol::AskForApproval; use codex_core::read_session_meta_line; @@ -288,6 +290,19 @@ pub async fn run_main( cloud_requirements.clone(), ) .await; + + #[allow(clippy::print_stderr)] + match check_execpolicy_for_warnings(&config.config_layer_stack).await { + Ok(None) => {} + Ok(Some(err)) | Err(err) => { + eprintln!( + "Error loading rules:\n{}", + format_exec_policy_error_with_source(&err) + ); + std::process::exit(1); + } + } + set_default_client_residency_requirement(config.enforce_residency.value()); if let Some(warning) =