mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
Merge d022fb53fd into sapling-pr-archive-bolinfest
This commit is contained in:
Generated
+4
@@ -1470,6 +1470,7 @@ dependencies = [
|
||||
"codex-arg0",
|
||||
"codex-chatgpt",
|
||||
"codex-cloud-tasks",
|
||||
"codex-config",
|
||||
"codex-core",
|
||||
"codex-exec",
|
||||
"codex-execpolicy",
|
||||
@@ -1600,10 +1601,13 @@ dependencies = [
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serde_path_to_error",
|
||||
"sha2",
|
||||
"thiserror 2.0.18",
|
||||
"tokio",
|
||||
"toml 0.9.11+spec-1.1.0",
|
||||
"toml_edit 0.24.0+spec-1.1.0",
|
||||
"tracing",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -847,7 +847,9 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> {
|
||||
// Approve and wait for task completion
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::json!({ "decision": codex_core::protocol::ReviewDecision::Approved }),
|
||||
serde_json::to_value(CommandExecutionRequestApprovalResponse {
|
||||
decision: CommandExecutionApprovalDecision::Accept,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
timeout(
|
||||
@@ -1035,7 +1037,7 @@ async fn turn_start_exec_approval_decline_v2() -> Result<()> {
|
||||
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
@@ -1129,7 +1131,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> {
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
mcp.clear_message_buffer();
|
||||
|
||||
@@ -26,6 +26,7 @@ codex-arg0 = { workspace = true }
|
||||
codex-chatgpt = { workspace = true }
|
||||
codex-cloud-tasks = { path = "../cloud-tasks" }
|
||||
codex-utils-cli = { workspace = true }
|
||||
codex-config = { workspace = true }
|
||||
codex-core = { workspace = true }
|
||||
codex-exec = { workspace = true }
|
||||
codex-execpolicy = { workspace = true }
|
||||
|
||||
@@ -870,7 +870,7 @@ fn maybe_print_under_development_feature_warning(
|
||||
return;
|
||||
}
|
||||
|
||||
let config_path = codex_home.join(codex_core::config::CONFIG_TOML_FILE);
|
||||
let config_path = codex_home.join(codex_config::CONFIG_TOML_FILE);
|
||||
eprintln!(
|
||||
"Under-development features enabled: {feature}. Under-development features are incomplete and may behave unpredictably. To suppress this warning, set `suppress_unstable_features_warning = true` in {}.",
|
||||
config_path.display()
|
||||
|
||||
@@ -16,9 +16,13 @@ futures = { workspace = true, features = ["alloc", "std"] }
|
||||
multimap = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_path_to_error = { workspace = true }
|
||||
sha2 = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
tokio = { workspace = true, features = ["fs"] }
|
||||
toml = { workspace = true }
|
||||
toml_edit = { workspace = true }
|
||||
tracing = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
anyhow = { workspace = true }
|
||||
|
||||
+25
-19
@@ -1,10 +1,12 @@
|
||||
//! Helpers for mapping config parse/validation failures to file locations and
|
||||
//! rendering them in a user-friendly way.
|
||||
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::ConfigToml;
|
||||
use crate::ConfigLayerEntry;
|
||||
use crate::ConfigLayerStack;
|
||||
use crate::ConfigLayerStackOrdering;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use serde::de::DeserializeOwned;
|
||||
use serde_path_to_error::Path as SerdePath;
|
||||
use serde_path_to_error::Segment as SerdeSegment;
|
||||
use std::fmt;
|
||||
@@ -17,10 +19,6 @@ use toml_edit::Item;
|
||||
use toml_edit::Table;
|
||||
use toml_edit::Value;
|
||||
|
||||
use super::ConfigLayerEntry;
|
||||
use super::ConfigLayerStack;
|
||||
use super::ConfigLayerStackOrdering;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct TextPosition {
|
||||
pub line: usize,
|
||||
@@ -88,7 +86,7 @@ impl std::error::Error for ConfigLoadError {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn io_error_from_config_error(
|
||||
pub fn io_error_from_config_error(
|
||||
kind: io::ErrorKind,
|
||||
error: ConfigError,
|
||||
source: Option<toml::de::Error>,
|
||||
@@ -96,7 +94,7 @@ pub(crate) fn io_error_from_config_error(
|
||||
io::Error::new(kind, ConfigLoadError::new(error, source))
|
||||
}
|
||||
|
||||
pub(crate) fn config_error_from_toml(
|
||||
pub fn config_error_from_toml(
|
||||
path: impl AsRef<Path>,
|
||||
contents: &str,
|
||||
err: toml::de::Error,
|
||||
@@ -108,7 +106,7 @@ pub(crate) fn config_error_from_toml(
|
||||
ConfigError::new(path.as_ref().to_path_buf(), range, err.message())
|
||||
}
|
||||
|
||||
pub(crate) fn config_error_from_config_toml(
|
||||
pub fn config_error_from_typed_toml<T: DeserializeOwned>(
|
||||
path: impl AsRef<Path>,
|
||||
contents: &str,
|
||||
) -> Option<ConfigError> {
|
||||
@@ -117,7 +115,7 @@ pub(crate) fn config_error_from_config_toml(
|
||||
Err(err) => return Some(config_error_from_toml(path, contents, err)),
|
||||
};
|
||||
|
||||
let result: Result<ConfigToml, _> = serde_path_to_error::deserialize(deserializer);
|
||||
let result: Result<T, _> = serde_path_to_error::deserialize(deserializer);
|
||||
match result {
|
||||
Ok(_) => None,
|
||||
Err(err) => {
|
||||
@@ -136,28 +134,36 @@ pub(crate) fn config_error_from_config_toml(
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn first_layer_config_error(layers: &ConfigLayerStack) -> Option<ConfigError> {
|
||||
pub async fn first_layer_config_error<T: DeserializeOwned>(
|
||||
layers: &ConfigLayerStack,
|
||||
config_toml_file: &str,
|
||||
) -> Option<ConfigError> {
|
||||
// When the merged config fails schema validation, we surface the first concrete
|
||||
// per-file error to point users at a specific file and range rather than an
|
||||
// opaque merged-layer failure.
|
||||
first_layer_config_error_for_entries(
|
||||
first_layer_config_error_for_entries::<T, _>(
|
||||
layers.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false),
|
||||
config_toml_file,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn first_layer_config_error_from_entries(
|
||||
pub async fn first_layer_config_error_from_entries<T: DeserializeOwned>(
|
||||
layers: &[ConfigLayerEntry],
|
||||
config_toml_file: &str,
|
||||
) -> Option<ConfigError> {
|
||||
first_layer_config_error_for_entries(layers.iter()).await
|
||||
first_layer_config_error_for_entries::<T, _>(layers.iter(), config_toml_file).await
|
||||
}
|
||||
|
||||
async fn first_layer_config_error_for_entries<'a, I>(layers: I) -> Option<ConfigError>
|
||||
async fn first_layer_config_error_for_entries<'a, T: DeserializeOwned, I>(
|
||||
layers: I,
|
||||
config_toml_file: &str,
|
||||
) -> Option<ConfigError>
|
||||
where
|
||||
I: IntoIterator<Item = &'a ConfigLayerEntry>,
|
||||
{
|
||||
for layer in layers {
|
||||
let Some(path) = config_path_for_layer(layer) else {
|
||||
let Some(path) = config_path_for_layer(layer, config_toml_file) else {
|
||||
continue;
|
||||
};
|
||||
let contents = match tokio::fs::read_to_string(&path).await {
|
||||
@@ -174,7 +180,7 @@ where
|
||||
continue;
|
||||
};
|
||||
let _guard = AbsolutePathBufGuard::new(parent);
|
||||
if let Some(error) = config_error_from_config_toml(&path, &contents) {
|
||||
if let Some(error) = config_error_from_typed_toml::<T>(&path, &contents) {
|
||||
return Some(error);
|
||||
}
|
||||
}
|
||||
@@ -182,12 +188,12 @@ where
|
||||
None
|
||||
}
|
||||
|
||||
fn config_path_for_layer(layer: &ConfigLayerEntry) -> Option<PathBuf> {
|
||||
fn config_path_for_layer(layer: &ConfigLayerEntry, config_toml_file: &str) -> Option<PathBuf> {
|
||||
match &layer.name {
|
||||
ConfigLayerSource::System { file } => Some(file.to_path_buf()),
|
||||
ConfigLayerSource::User { file } => Some(file.to_path_buf()),
|
||||
ConfigLayerSource::Project { dot_codex_folder } => {
|
||||
Some(dot_codex_folder.as_path().join(CONFIG_TOML_FILE))
|
||||
Some(dot_codex_folder.as_path().join(config_toml_file))
|
||||
}
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => Some(file.to_path_buf()),
|
||||
ConfigLayerSource::Mdm { .. }
|
||||
@@ -1,12 +1,15 @@
|
||||
mod cloud_requirements;
|
||||
mod config_requirements;
|
||||
mod constraint;
|
||||
mod diagnostics;
|
||||
mod fingerprint;
|
||||
mod merge;
|
||||
mod overrides;
|
||||
mod requirements_exec_policy;
|
||||
mod state;
|
||||
|
||||
pub const CONFIG_TOML_FILE: &str = "config.toml";
|
||||
|
||||
pub use cloud_requirements::CloudRequirementsLoader;
|
||||
pub use config_requirements::ConfigRequirements;
|
||||
pub use config_requirements::ConfigRequirementsToml;
|
||||
@@ -24,6 +27,17 @@ pub use config_requirements::WebSearchModeRequirement;
|
||||
pub use constraint::Constrained;
|
||||
pub use constraint::ConstraintError;
|
||||
pub use constraint::ConstraintResult;
|
||||
pub use diagnostics::ConfigError;
|
||||
pub use diagnostics::ConfigLoadError;
|
||||
pub use diagnostics::TextPosition;
|
||||
pub use diagnostics::TextRange;
|
||||
pub use diagnostics::config_error_from_toml;
|
||||
pub use diagnostics::config_error_from_typed_toml;
|
||||
pub use diagnostics::first_layer_config_error;
|
||||
pub use diagnostics::first_layer_config_error_from_entries;
|
||||
pub use diagnostics::format_config_error;
|
||||
pub use diagnostics::format_config_error_with_source;
|
||||
pub use diagnostics::io_error_from_config_error;
|
||||
pub use fingerprint::version_for_toml;
|
||||
pub use merge::merge_toml_values;
|
||||
pub use overrides::build_cli_overrides_layer;
|
||||
|
||||
@@ -119,7 +119,6 @@ use crate::client_common::Prompt;
|
||||
use crate::client_common::ResponseEvent;
|
||||
use crate::codex_thread::ThreadConfigSnapshot;
|
||||
use crate::compact::collect_user_messages;
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::Config;
|
||||
use crate::config::Constrained;
|
||||
use crate::config::ConstraintResult;
|
||||
@@ -135,6 +134,7 @@ use crate::error::CodexErr;
|
||||
use crate::error::Result as CodexResult;
|
||||
#[cfg(test)]
|
||||
use crate::exec::StreamOutput;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
|
||||
#[derive(Debug, PartialEq)]
|
||||
pub enum SteerInputError {
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::types::McpServerConfig;
|
||||
use crate::config::types::Notice;
|
||||
use crate::path_utils::resolve_symlink_write_paths;
|
||||
use crate::path_utils::write_atomically;
|
||||
use anyhow::Context;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
|
||||
@@ -117,8 +117,6 @@ pub(crate) const PROJECT_DOC_MAX_BYTES: usize = 32 * 1024; // 32 KiB
|
||||
pub(crate) const DEFAULT_AGENT_MAX_THREADS: Option<usize> = Some(6);
|
||||
pub(crate) const DEFAULT_AGENT_MAX_DEPTH: i32 = 1;
|
||||
|
||||
pub const CONFIG_TOML_FILE: &str = "config.toml";
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn test_config() -> Config {
|
||||
let codex_home = tempdir().expect("create temp dir");
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use super::CONFIG_TOML_FILE;
|
||||
use super::ConfigToml;
|
||||
use crate::config::edit::ConfigEdit;
|
||||
use crate::config::edit::ConfigEditsBuilder;
|
||||
@@ -26,6 +25,7 @@ use codex_app_server_protocol::ConfigWriteResponse;
|
||||
use codex_app_server_protocol::MergeStrategy;
|
||||
use codex_app_server_protocol::OverriddenMetadata;
|
||||
use codex_app_server_protocol::WriteStatus;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::borrow::Cow;
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
use super::LoaderOverrides;
|
||||
use super::diagnostics::config_error_from_toml;
|
||||
use super::diagnostics::io_error_from_config_error;
|
||||
#[cfg(target_os = "macos")]
|
||||
use super::macos::ManagedAdminConfigLayer;
|
||||
#[cfg(target_os = "macos")]
|
||||
use super::macos::load_managed_admin_config_layer;
|
||||
use codex_config::config_error_from_toml;
|
||||
use codex_config::io_error_from_config_error;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
mod diagnostics;
|
||||
mod layer_io;
|
||||
#[cfg(target_os = "macos")]
|
||||
mod macos;
|
||||
@@ -6,12 +5,12 @@ mod macos;
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::ConfigToml;
|
||||
use crate::config::deserialize_config_toml_with_base;
|
||||
use crate::config_loader::layer_io::LoadedConfigLayers;
|
||||
use crate::git_info::resolve_root_git_project_for_trust;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigRequirementsWithSources;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
@@ -27,9 +26,11 @@ use std::path::PathBuf;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
pub use codex_config::CloudRequirementsLoader;
|
||||
pub use codex_config::ConfigError;
|
||||
pub use codex_config::ConfigLayerEntry;
|
||||
pub use codex_config::ConfigLayerStack;
|
||||
pub use codex_config::ConfigLayerStackOrdering;
|
||||
pub use codex_config::ConfigLoadError;
|
||||
pub use codex_config::ConfigRequirements;
|
||||
pub use codex_config::ConfigRequirementsToml;
|
||||
pub use codex_config::ConstrainedWithSource;
|
||||
@@ -42,21 +43,17 @@ pub use codex_config::RequirementSource;
|
||||
pub use codex_config::ResidencyRequirement;
|
||||
pub use codex_config::SandboxModeRequirement;
|
||||
pub use codex_config::Sourced;
|
||||
pub use codex_config::TextPosition;
|
||||
pub use codex_config::TextRange;
|
||||
pub use codex_config::WebSearchModeRequirement;
|
||||
pub(crate) use codex_config::build_cli_overrides_layer;
|
||||
pub(crate) use codex_config::config_error_from_toml;
|
||||
pub use codex_config::format_config_error;
|
||||
pub use codex_config::format_config_error_with_source;
|
||||
pub(crate) use codex_config::io_error_from_config_error;
|
||||
pub use codex_config::merge_toml_values;
|
||||
#[cfg(test)]
|
||||
pub(crate) use codex_config::version_for_toml;
|
||||
pub use diagnostics::ConfigError;
|
||||
pub use diagnostics::ConfigLoadError;
|
||||
pub use diagnostics::TextPosition;
|
||||
pub use diagnostics::TextRange;
|
||||
pub(crate) use diagnostics::config_error_from_toml;
|
||||
pub(crate) use diagnostics::first_layer_config_error;
|
||||
pub(crate) use diagnostics::first_layer_config_error_from_entries;
|
||||
pub use diagnostics::format_config_error;
|
||||
pub use diagnostics::format_config_error_with_source;
|
||||
pub(crate) use diagnostics::io_error_from_config_error;
|
||||
|
||||
/// On Unix systems, load default settings from this file path, if present.
|
||||
/// Note that /etc/codex/ is treated as a "config folder," so subfolders such
|
||||
@@ -68,6 +65,17 @@ const DEFAULT_PROGRAM_DATA_DIR_WINDOWS: &str = r"C:\ProgramData";
|
||||
|
||||
const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"];
|
||||
|
||||
pub(crate) async fn first_layer_config_error(layers: &ConfigLayerStack) -> Option<ConfigError> {
|
||||
codex_config::first_layer_config_error::<ConfigToml>(layers, CONFIG_TOML_FILE).await
|
||||
}
|
||||
|
||||
pub(crate) async fn first_layer_config_error_from_entries(
|
||||
layers: &[ConfigLayerEntry],
|
||||
) -> Option<ConfigError> {
|
||||
codex_config::first_layer_config_error_from_entries::<ConfigToml>(layers, CONFIG_TOML_FILE)
|
||||
.await
|
||||
}
|
||||
|
||||
/// To build up the set of admin-enforced constraints, we build up from multiple
|
||||
/// configuration layers in the following order, but a constraint defined in an
|
||||
/// earlier layer cannot be overridden by a later layer:
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
use super::LoaderOverrides;
|
||||
use super::load_config_layers_state;
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::ConfigToml;
|
||||
@@ -15,6 +14,7 @@ use crate::config_loader::ConfigRequirementsWithSources;
|
||||
use crate::config_loader::RequirementSource;
|
||||
use crate::config_loader::load_requirements_toml;
|
||||
use crate::config_loader::version_for_toml;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -153,7 +153,7 @@ async fn returns_config_error_for_schema_error_in_user_config() {
|
||||
let config_error = config_error_from_io(&err);
|
||||
let _guard = codex_utils_absolute_path::AbsolutePathBufGuard::new(tmp.path());
|
||||
let expected_config_error =
|
||||
super::diagnostics::config_error_from_config_toml(&config_path, contents)
|
||||
codex_config::config_error_from_typed_toml::<ConfigToml>(&config_path, contents)
|
||||
.expect("schema error");
|
||||
assert_eq!(config_error, &expected_config_error);
|
||||
}
|
||||
@@ -166,7 +166,7 @@ fn schema_error_points_to_feature_value() {
|
||||
std::fs::write(&config_path, contents).expect("write config");
|
||||
|
||||
let _guard = codex_utils_absolute_path::AbsolutePathBufGuard::new(tmp.path());
|
||||
let error = super::diagnostics::config_error_from_config_toml(&config_path, contents)
|
||||
let error = codex_config::config_error_from_typed_toml::<ConfigToml>(&config_path, contents)
|
||||
.expect("schema error");
|
||||
|
||||
let value_line = contents.lines().nth(1).expect("value line");
|
||||
|
||||
@@ -224,6 +224,9 @@ impl ExecPolicyManager {
|
||||
let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
prefix_rule.as_ref(),
|
||||
&evaluation.matched_rules,
|
||||
exec_policy.as_ref(),
|
||||
&commands,
|
||||
&exec_policy_fallback,
|
||||
);
|
||||
|
||||
match evaluation.decision {
|
||||
@@ -592,6 +595,9 @@ fn try_derive_execpolicy_amendment_for_allow_rules(
|
||||
fn derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
prefix_rule: Option<&Vec<String>>,
|
||||
matched_rules: &[RuleMatch],
|
||||
exec_policy: &Policy,
|
||||
commands: &[Vec<String>],
|
||||
exec_policy_fallback: &impl Fn(&[String]) -> Decision,
|
||||
) -> Option<ExecPolicyAmendment> {
|
||||
let prefix_rule = prefix_rule?;
|
||||
if prefix_rule.is_empty() {
|
||||
@@ -612,7 +618,39 @@ fn derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(ExecPolicyAmendment::new(prefix_rule.clone()))
|
||||
let amendment = ExecPolicyAmendment::new(prefix_rule.clone());
|
||||
if prefix_rule_would_approve_all_commands(
|
||||
exec_policy,
|
||||
&amendment.command,
|
||||
commands,
|
||||
exec_policy_fallback,
|
||||
) {
|
||||
Some(amendment)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn prefix_rule_would_approve_all_commands(
|
||||
exec_policy: &Policy,
|
||||
prefix_rule: &[String],
|
||||
commands: &[Vec<String>],
|
||||
exec_policy_fallback: &impl Fn(&[String]) -> Decision,
|
||||
) -> bool {
|
||||
let mut policy_with_prefix_rule = exec_policy.clone();
|
||||
if policy_with_prefix_rule
|
||||
.add_prefix_rule(prefix_rule, Decision::Allow)
|
||||
.is_err()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
commands.iter().all(|command| {
|
||||
policy_with_prefix_rule
|
||||
.check(command, exec_policy_fallback)
|
||||
.decision
|
||||
== Decision::Allow
|
||||
})
|
||||
}
|
||||
|
||||
/// Only return a reason when a policy rule drove the prompt decision.
|
||||
@@ -1125,7 +1163,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn keeps_requested_amendment_for_heredoc_fallback_prompts() {
|
||||
async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_wont_match() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
@@ -1147,7 +1185,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(requested_prefix)),
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -1472,6 +1510,38 @@ prefix_rule(
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"cargo install cargo-insta && rm -rf /tmp/codex".to_string(),
|
||||
];
|
||||
let manager = ExecPolicyManager::default();
|
||||
|
||||
let requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"rm".to_string(),
|
||||
"-rf".to_string(),
|
||||
"/tmp/codex".to_string(),
|
||||
])),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn heuristics_apply_when_other_commands_match_policy() {
|
||||
let policy_src = r#"prefix_rule(pattern=["apple"], decision="allow")"#;
|
||||
@@ -1728,11 +1798,28 @@ prefix_rule(
|
||||
);
|
||||
}
|
||||
|
||||
fn derive_requested_execpolicy_amendment_for_test(
|
||||
prefix_rule: Option<&Vec<String>>,
|
||||
matched_rules: &[RuleMatch],
|
||||
) -> Option<ExecPolicyAmendment> {
|
||||
let commands = prefix_rule
|
||||
.cloned()
|
||||
.map(|prefix_rule| vec![prefix_rule])
|
||||
.unwrap_or_else(|| vec![vec!["echo".to_string()]]);
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
prefix_rule,
|
||||
matched_rules,
|
||||
&Policy::empty(),
|
||||
&commands,
|
||||
&|_: &[String]| Decision::Allow,
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_requested_execpolicy_amendment_returns_none_for_missing_prefix_rule() {
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(None, &[])
|
||||
derive_requested_execpolicy_amendment_for_test(None, &[])
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1740,7 +1827,7 @@ prefix_rule(
|
||||
fn derive_requested_execpolicy_amendment_returns_none_for_empty_prefix_rule() {
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&Vec::new()), &[])
|
||||
derive_requested_execpolicy_amendment_for_test(Some(&Vec::new()), &[])
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1748,7 +1835,7 @@ prefix_rule(
|
||||
fn derive_requested_execpolicy_amendment_returns_none_for_exact_banned_prefix_rule() {
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
derive_requested_execpolicy_amendment_for_test(
|
||||
Some(&vec!["python".to_string(), "-c".to_string()]),
|
||||
&[],
|
||||
)
|
||||
@@ -1767,7 +1854,7 @@ prefix_rule(
|
||||
] {
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[])
|
||||
derive_requested_execpolicy_amendment_for_test(Some(&prefix_rule), &[])
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1793,7 +1880,7 @@ prefix_rule(
|
||||
] {
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[])
|
||||
derive_requested_execpolicy_amendment_for_test(Some(&prefix_rule), &[])
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1808,7 +1895,7 @@ prefix_rule(
|
||||
|
||||
assert_eq!(
|
||||
Some(ExecPolicyAmendment::new(prefix_rule.clone())),
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[])
|
||||
derive_requested_execpolicy_amendment_for_test(Some(&prefix_rule), &[])
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1823,7 +1910,7 @@ prefix_rule(
|
||||
}];
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
derive_requested_execpolicy_amendment_for_test(
|
||||
Some(&prefix_rule),
|
||||
&matched_rules_prompt
|
||||
),
|
||||
@@ -1836,7 +1923,7 @@ prefix_rule(
|
||||
}];
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
derive_requested_execpolicy_amendment_for_test(
|
||||
Some(&prefix_rule),
|
||||
&matched_rules_allow
|
||||
),
|
||||
@@ -1849,9 +1936,9 @@ prefix_rule(
|
||||
}];
|
||||
assert_eq!(
|
||||
None,
|
||||
derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
derive_requested_execpolicy_amendment_for_test(
|
||||
Some(&prefix_rule),
|
||||
&matched_rules_forbidden
|
||||
&matched_rules_forbidden,
|
||||
),
|
||||
"should return none when prompt policy matches"
|
||||
);
|
||||
|
||||
@@ -5,13 +5,13 @@
|
||||
//! booleans through multiple types, call sites consult a single `Features`
|
||||
//! container attached to `Config`.
|
||||
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::Config;
|
||||
use crate::config::ConfigToml;
|
||||
use crate::config::profile::ConfigProfile;
|
||||
use crate::protocol::Event;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::WarningEvent;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_otel::OtelManager;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::NetworkToml;
|
||||
use crate::config::PermissionsToml;
|
||||
use crate::config::find_codex_home;
|
||||
@@ -11,6 +10,7 @@ use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use async_trait::async_trait;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_network_proxy::ConfigReloader;
|
||||
use codex_network_proxy::ConfigState;
|
||||
use codex_network_proxy::NetworkProxyConfig;
|
||||
|
||||
@@ -806,7 +806,6 @@ fn extract_frontmatter(contents: &str) -> Option<String> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::ConfigToml;
|
||||
@@ -817,6 +816,7 @@ mod tests {
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
@@ -184,6 +184,16 @@ fn shell_event(
|
||||
command: &str,
|
||||
timeout_ms: u64,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
) -> Result<Value> {
|
||||
shell_event_with_prefix_rule(call_id, command, timeout_ms, sandbox_permissions, None)
|
||||
}
|
||||
|
||||
fn shell_event_with_prefix_rule(
|
||||
call_id: &str,
|
||||
command: &str,
|
||||
timeout_ms: u64,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
prefix_rule: Option<Vec<String>>,
|
||||
) -> Result<Value> {
|
||||
let mut args = json!({
|
||||
"command": command,
|
||||
@@ -192,6 +202,9 @@ fn shell_event(
|
||||
if sandbox_permissions.requires_escalated_permissions() {
|
||||
args["sandbox_permissions"] = json!(sandbox_permissions);
|
||||
}
|
||||
if let Some(prefix_rule) = prefix_rule {
|
||||
args["prefix_rule"] = json!(prefix_rule);
|
||||
}
|
||||
let args_str = serde_json::to_string(&args)?;
|
||||
Ok(ev_function_call(call_id, "shell_command", &args_str))
|
||||
}
|
||||
@@ -1928,6 +1941,171 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn invalid_requested_prefix_rule_falls_back_for_compound_command() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let sandbox_policy_for_config = sandbox_policy.clone();
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "invalid-prefix-rule";
|
||||
let command =
|
||||
"touch /tmp/codex-fallback-rule-test.txt && echo hello > /tmp/codex-fallback-rule-test.txt";
|
||||
let event = shell_event_with_prefix_rule(
|
||||
call_id,
|
||||
command,
|
||||
1_000,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
Some(vec!["touch".to_string()]),
|
||||
)?;
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-invalid-prefix-1"),
|
||||
event,
|
||||
ev_completed("resp-invalid-prefix-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"invalid-prefix-rule",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let approval = expect_exec_approval(&test, command).await;
|
||||
let amendment = approval
|
||||
.proposed_execpolicy_amendment
|
||||
.expect("should have a proposed execpolicy amendment");
|
||||
assert!(amendment.command.contains(&command.to_string()));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn approving_fallback_rule_for_compound_command_works() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let sandbox_policy_for_config = sandbox_policy.clone();
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "invalid-prefix-rule";
|
||||
let command =
|
||||
"touch /tmp/codex-fallback-rule-test.txt && echo hello > /tmp/codex-fallback-rule-test.txt";
|
||||
let event = shell_event_with_prefix_rule(
|
||||
call_id,
|
||||
command,
|
||||
1_000,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
Some(vec!["touch".to_string()]),
|
||||
)?;
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-invalid-prefix-1"),
|
||||
event,
|
||||
ev_completed("resp-invalid-prefix-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"invalid-prefix-rule",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let approval = expect_exec_approval(&test, command).await;
|
||||
let approval_id = approval.effective_approval_id();
|
||||
let amendment = approval
|
||||
.proposed_execpolicy_amendment
|
||||
.expect("should have a proposed execpolicy amendment");
|
||||
assert!(amendment.command.contains(&command.to_string()));
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval_id,
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment: amendment.clone(),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion(&test).await;
|
||||
|
||||
let call_id = "invalid-prefix-rule-again";
|
||||
let command =
|
||||
"touch /tmp/codex-fallback-rule-test.txt && echo hello > /tmp/codex-fallback-rule-test.txt";
|
||||
let event = shell_event_with_prefix_rule(
|
||||
call_id,
|
||||
command,
|
||||
1_000,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
Some(vec!["touch".to_string()]),
|
||||
)?;
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-invalid-prefix-1"),
|
||||
event,
|
||||
ev_completed("resp-invalid-prefix-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let second_results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-invalid-prefix-1", "done"),
|
||||
ev_completed("resp-invalid-prefix-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"invalid-prefix-rule",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
wait_for_completion_without_approval(&test).await;
|
||||
|
||||
let second_output = parse_result(
|
||||
&second_results
|
||||
.single_request()
|
||||
.function_call_output(call_id),
|
||||
);
|
||||
assert_eq!(second_output.exit_code.unwrap_or(0), 0);
|
||||
assert!(
|
||||
second_output.stdout.is_empty(),
|
||||
"unexpected stdout: {}",
|
||||
second_output.stdout
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// todo(dylan) add ScenarioSpec support for rules
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_core::CodexAuth;
|
||||
use codex_core::NewThread;
|
||||
use codex_core::config::CONFIG_TOML_FILE;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::InitialHistory;
|
||||
|
||||
Reference in New Issue
Block a user