mirror of
https://github.com/openai/codex.git
synced 2026-11-08 00:36:31 +00:00
feat: load ExecPolicyManager from ConfigLayerStack (#8453)
https://github.com/openai/codex/pull/8354 added support for in-repo `.config/` files, so this PR updates the logic for loading `*.rules` files to load `*.rules` files from all relevant layers. The main change to the business logic is `load_exec_policy()` in `codex-rs/core/src/exec_policy.rs`. Note this adds a `config_folder()` method to `ConfigLayerSource` that returns `Option<AbsolutePathBuf>` so that it is straightforward to iterate over the sources and get the associated config folder, if any.
This commit is contained in:
1 parent
14dbd0610a
commit
277babba79
6 files changed
+166
-19
No files matched your search
@@ -240,7 +240,7 @@ impl Codex {
|
|||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
let exec_policy = ExecPolicyManager::load(&config.features, &config.codex_home)
|
let exec_policy = ExecPolicyManager::load(&config.features, &config.config_layer_stack)
|
||||||
.await
|
.await
|
||||||
.map_err(|err| CodexErr::Fatal(format!("failed to load execpolicy: {err}")))?;
|
.map_err(|err| CodexErr::Fatal(format!("failed to load execpolicy: {err}")))?;
|
||||||
|
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ pub use config_requirements::ConfigRequirements;
|
|||||||
pub use merge::merge_toml_values;
|
pub use merge::merge_toml_values;
|
||||||
pub use state::ConfigLayerEntry;
|
pub use state::ConfigLayerEntry;
|
||||||
pub use state::ConfigLayerStack;
|
pub use state::ConfigLayerStack;
|
||||||
|
pub use state::ConfigLayerStackOrdering;
|
||||||
pub use state::LoaderOverrides;
|
pub use state::LoaderOverrides;
|
||||||
|
|
||||||
/// On Unix systems, load requirements from this file path, if present.
|
/// On Unix systems, load requirements from this file path, if present.
|
||||||
|
|||||||
@@ -50,6 +50,25 @@ impl ConfigLayerEntry {
|
|||||||
config: serde_json::to_value(&self.config).unwrap_or(JsonValue::Null),
|
config: serde_json::to_value(&self.config).unwrap_or(JsonValue::Null),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Get the `.codex/` folder associated with this config layer, if any.
|
||||||
|
pub fn config_folder(&self) -> Option<AbsolutePathBuf> {
|
||||||
|
match &self.name {
|
||||||
|
ConfigLayerSource::Mdm { .. } => None,
|
||||||
|
ConfigLayerSource::System { .. } => None,
|
||||||
|
ConfigLayerSource::User { file } => file.parent(),
|
||||||
|
ConfigLayerSource::Project { dot_codex_folder } => Some(dot_codex_folder.clone()),
|
||||||
|
ConfigLayerSource::SessionFlags => None,
|
||||||
|
ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } => None,
|
||||||
|
ConfigLayerSource::LegacyManagedConfigTomlFromMdm => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
|
pub enum ConfigLayerStackOrdering {
|
||||||
|
LowestPrecedenceFirst,
|
||||||
|
HighestPrecedenceFirst,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Default, PartialEq)]
|
#[derive(Debug, Clone, Default, PartialEq)]
|
||||||
@@ -156,7 +175,16 @@ impl ConfigLayerStack {
|
|||||||
/// Returns the highest-precedence to lowest-precedence layers, so
|
/// Returns the highest-precedence to lowest-precedence layers, so
|
||||||
/// `ConfigLayerSource::SessionFlags` would be first, if present.
|
/// `ConfigLayerSource::SessionFlags` would be first, if present.
|
||||||
pub fn layers_high_to_low(&self) -> Vec<&ConfigLayerEntry> {
|
pub fn layers_high_to_low(&self) -> Vec<&ConfigLayerEntry> {
|
||||||
self.layers.iter().rev().collect()
|
self.get_layers(ConfigLayerStackOrdering::HighestPrecedenceFirst)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns the highest-precedence to lowest-precedence layers, so
|
||||||
|
/// `ConfigLayerSource::SessionFlags` would be first, if present.
|
||||||
|
pub fn get_layers(&self, ordering: ConfigLayerStackOrdering) -> Vec<&ConfigLayerEntry> {
|
||||||
|
match ordering {
|
||||||
|
ConfigLayerStackOrdering::HighestPrecedenceFirst => self.layers.iter().rev().collect(),
|
||||||
|
ConfigLayerStackOrdering::LowestPrecedenceFirst => self.layers.iter().collect(),
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ use std::sync::Arc;
|
|||||||
use arc_swap::ArcSwap;
|
use arc_swap::ArcSwap;
|
||||||
|
|
||||||
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
|
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
|
||||||
|
use crate::config_loader::ConfigLayerStack;
|
||||||
|
use crate::config_loader::ConfigLayerStackOrdering;
|
||||||
use codex_execpolicy::AmendError;
|
use codex_execpolicy::AmendError;
|
||||||
use codex_execpolicy::Decision;
|
use codex_execpolicy::Decision;
|
||||||
use codex_execpolicy::Error as ExecPolicyRuleError;
|
use codex_execpolicy::Error as ExecPolicyRuleError;
|
||||||
@@ -94,9 +96,9 @@ impl ExecPolicyManager {
|
|||||||
|
|
||||||
pub(crate) async fn load(
|
pub(crate) async fn load(
|
||||||
features: &Features,
|
features: &Features,
|
||||||
codex_home: &Path,
|
config_stack: &ConfigLayerStack,
|
||||||
) -> Result<Self, ExecPolicyError> {
|
) -> Result<Self, ExecPolicyError> {
|
||||||
let policy = load_exec_policy_for_features(features, codex_home).await?;
|
let policy = load_exec_policy_for_features(features, config_stack).await?;
|
||||||
Ok(Self::new(Arc::new(policy)))
|
Ok(Self::new(Arc::new(policy)))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -194,18 +196,28 @@ impl Default for ExecPolicyManager {
|
|||||||
|
|
||||||
async fn load_exec_policy_for_features(
|
async fn load_exec_policy_for_features(
|
||||||
features: &Features,
|
features: &Features,
|
||||||
codex_home: &Path,
|
config_stack: &ConfigLayerStack,
|
||||||
) -> Result<Policy, ExecPolicyError> {
|
) -> Result<Policy, ExecPolicyError> {
|
||||||
if !features.enabled(Feature::ExecPolicy) {
|
if !features.enabled(Feature::ExecPolicy) {
|
||||||
Ok(Policy::empty())
|
Ok(Policy::empty())
|
||||||
} else {
|
} else {
|
||||||
load_exec_policy(codex_home).await
|
load_exec_policy(config_stack).await
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub async fn load_exec_policy(codex_home: &Path) -> Result<Policy, ExecPolicyError> {
|
pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result<Policy, ExecPolicyError> {
|
||||||
let policy_dir = codex_home.join(RULES_DIR_NAME);
|
// Iterate the layers in increasing order of precedence, adding the *.rules
|
||||||
let policy_paths = collect_policy_files(&policy_dir).await?;
|
// from each layer, so that higher-precedence layers can override
|
||||||
|
// rules defined in lower-precedence ones.
|
||||||
|
let mut policy_paths = Vec::new();
|
||||||
|
for layer in config_stack.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst) {
|
||||||
|
if let Some(config_folder) = layer.config_folder() {
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
|
let policy_dir = config_folder.join(RULES_DIR_NAME).expect("safe join");
|
||||||
|
let layer_policy_paths = collect_policy_files(&policy_dir).await?;
|
||||||
|
policy_paths.extend(layer_policy_paths);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
let mut parser = PolicyParser::new();
|
let mut parser = PolicyParser::new();
|
||||||
for policy_path in &policy_paths {
|
for policy_path in &policy_paths {
|
||||||
@@ -226,11 +238,7 @@ pub async fn load_exec_policy(codex_home: &Path) -> Result<Policy, ExecPolicyErr
|
|||||||
}
|
}
|
||||||
|
|
||||||
let policy = parser.build();
|
let policy = parser.build();
|
||||||
tracing::debug!(
|
tracing::debug!("loaded execpolicy from {} files", policy_paths.len());
|
||||||
"loaded execpolicy from {} files in {}",
|
|
||||||
policy_paths.len(),
|
|
||||||
policy_dir.display()
|
|
||||||
);
|
|
||||||
|
|
||||||
Ok(policy)
|
Ok(policy)
|
||||||
}
|
}
|
||||||
@@ -302,7 +310,8 @@ fn derive_prompt_reason(evaluation: &Evaluation) -> Option<String> {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn collect_policy_files(dir: &Path) -> Result<Vec<PathBuf>, ExecPolicyError> {
|
async fn collect_policy_files(dir: impl AsRef<Path>) -> Result<Vec<PathBuf>, ExecPolicyError> {
|
||||||
|
let dir = dir.as_ref();
|
||||||
let mut read_dir = match fs::read_dir(dir).await {
|
let mut read_dir = match fs::read_dir(dir).await {
|
||||||
Ok(read_dir) => read_dir,
|
Ok(read_dir) => read_dir,
|
||||||
Err(err) if err.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
|
Err(err) if err.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
|
||||||
@@ -345,28 +354,51 @@ async fn collect_policy_files(dir: &Path) -> Result<Vec<PathBuf>, ExecPolicyErro
|
|||||||
|
|
||||||
policy_paths.sort();
|
policy_paths.sort();
|
||||||
|
|
||||||
|
tracing::debug!(
|
||||||
|
"loaded {} .rules files in {}",
|
||||||
|
policy_paths.len(),
|
||||||
|
dir.display()
|
||||||
|
);
|
||||||
Ok(policy_paths)
|
Ok(policy_paths)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
use crate::config_loader::ConfigLayerEntry;
|
||||||
|
use crate::config_loader::ConfigLayerStack;
|
||||||
|
use crate::config_loader::ConfigRequirements;
|
||||||
use crate::features::Feature;
|
use crate::features::Feature;
|
||||||
use crate::features::Features;
|
use crate::features::Features;
|
||||||
|
use codex_app_server_protocol::ConfigLayerSource;
|
||||||
use codex_protocol::protocol::AskForApproval;
|
use codex_protocol::protocol::AskForApproval;
|
||||||
use codex_protocol::protocol::SandboxPolicy;
|
use codex_protocol::protocol::SandboxPolicy;
|
||||||
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
use std::fs;
|
use std::fs;
|
||||||
|
use std::path::Path;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use tempfile::tempdir;
|
use tempfile::tempdir;
|
||||||
|
use toml::Value as TomlValue;
|
||||||
|
|
||||||
|
fn config_stack_for_dot_codex_folder(dot_codex_folder: &Path) -> ConfigLayerStack {
|
||||||
|
let dot_codex_folder = AbsolutePathBuf::from_absolute_path(dot_codex_folder)
|
||||||
|
.expect("absolute dot_codex_folder");
|
||||||
|
let layer = ConfigLayerEntry::new(
|
||||||
|
ConfigLayerSource::Project { dot_codex_folder },
|
||||||
|
TomlValue::Table(Default::default()),
|
||||||
|
);
|
||||||
|
ConfigLayerStack::new(vec![layer], ConfigRequirements::default()).expect("ConfigLayerStack")
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn returns_empty_policy_when_feature_disabled() {
|
async fn returns_empty_policy_when_feature_disabled() {
|
||||||
let mut features = Features::with_defaults();
|
let mut features = Features::with_defaults();
|
||||||
features.disable(Feature::ExecPolicy);
|
features.disable(Feature::ExecPolicy);
|
||||||
let temp_dir = tempdir().expect("create temp dir");
|
let temp_dir = tempdir().expect("create temp dir");
|
||||||
|
let config_stack = config_stack_for_dot_codex_folder(temp_dir.path());
|
||||||
|
|
||||||
let manager = ExecPolicyManager::load(&features, temp_dir.path())
|
let manager = ExecPolicyManager::load(&features, &config_stack)
|
||||||
.await
|
.await
|
||||||
.expect("manager result");
|
.expect("manager result");
|
||||||
let policy = manager.current();
|
let policy = manager.current();
|
||||||
@@ -400,6 +432,7 @@ mod tests {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn loads_policies_from_policy_subdirectory() {
|
async fn loads_policies_from_policy_subdirectory() {
|
||||||
let temp_dir = tempdir().expect("create temp dir");
|
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);
|
let policy_dir = temp_dir.path().join(RULES_DIR_NAME);
|
||||||
fs::create_dir_all(&policy_dir).expect("create policy dir");
|
fs::create_dir_all(&policy_dir).expect("create policy dir");
|
||||||
fs::write(
|
fs::write(
|
||||||
@@ -408,7 +441,7 @@ mod tests {
|
|||||||
)
|
)
|
||||||
.expect("write policy file");
|
.expect("write policy file");
|
||||||
|
|
||||||
let policy = load_exec_policy(temp_dir.path())
|
let policy = load_exec_policy(&config_stack)
|
||||||
.await
|
.await
|
||||||
.expect("policy result");
|
.expect("policy result");
|
||||||
let command = [vec!["rm".to_string()]];
|
let command = [vec!["rm".to_string()]];
|
||||||
@@ -427,13 +460,14 @@ mod tests {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn ignores_policies_outside_policy_dir() {
|
async fn ignores_policies_outside_policy_dir() {
|
||||||
let temp_dir = tempdir().expect("create temp dir");
|
let temp_dir = tempdir().expect("create temp dir");
|
||||||
|
let config_stack = config_stack_for_dot_codex_folder(temp_dir.path());
|
||||||
fs::write(
|
fs::write(
|
||||||
temp_dir.path().join("root.rules"),
|
temp_dir.path().join("root.rules"),
|
||||||
r#"prefix_rule(pattern=["ls"], decision="prompt")"#,
|
r#"prefix_rule(pattern=["ls"], decision="prompt")"#,
|
||||||
)
|
)
|
||||||
.expect("write policy file");
|
.expect("write policy file");
|
||||||
|
|
||||||
let policy = load_exec_policy(temp_dir.path())
|
let policy = load_exec_policy(&config_stack)
|
||||||
.await
|
.await
|
||||||
.expect("policy result");
|
.expect("policy result");
|
||||||
let command = [vec!["ls".to_string()]];
|
let command = [vec!["ls".to_string()]];
|
||||||
@@ -449,6 +483,69 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn loads_policies_from_multiple_config_layers() -> anyhow::Result<()> {
|
||||||
|
let user_dir = tempdir()?;
|
||||||
|
let project_dir = tempdir()?;
|
||||||
|
|
||||||
|
let user_policy_dir = user_dir.path().join(RULES_DIR_NAME);
|
||||||
|
fs::create_dir_all(&user_policy_dir)?;
|
||||||
|
fs::write(
|
||||||
|
user_policy_dir.join("user.rules"),
|
||||||
|
r#"prefix_rule(pattern=["rm"], decision="forbidden")"#,
|
||||||
|
)?;
|
||||||
|
|
||||||
|
let project_policy_dir = project_dir.path().join(RULES_DIR_NAME);
|
||||||
|
fs::create_dir_all(&project_policy_dir)?;
|
||||||
|
fs::write(
|
||||||
|
project_policy_dir.join("project.rules"),
|
||||||
|
r#"prefix_rule(pattern=["ls"], decision="prompt")"#,
|
||||||
|
)?;
|
||||||
|
|
||||||
|
let user_config_toml =
|
||||||
|
AbsolutePathBuf::from_absolute_path(user_dir.path().join("config.toml"))?;
|
||||||
|
let project_dot_codex_folder = AbsolutePathBuf::from_absolute_path(project_dir.path())?;
|
||||||
|
let layers = vec![
|
||||||
|
ConfigLayerEntry::new(
|
||||||
|
ConfigLayerSource::User {
|
||||||
|
file: user_config_toml,
|
||||||
|
},
|
||||||
|
TomlValue::Table(Default::default()),
|
||||||
|
),
|
||||||
|
ConfigLayerEntry::new(
|
||||||
|
ConfigLayerSource::Project {
|
||||||
|
dot_codex_folder: project_dot_codex_folder,
|
||||||
|
},
|
||||||
|
TomlValue::Table(Default::default()),
|
||||||
|
),
|
||||||
|
];
|
||||||
|
let config_stack = ConfigLayerStack::new(layers, ConfigRequirements::default())?;
|
||||||
|
|
||||||
|
let policy = load_exec_policy(&config_stack).await?;
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
Evaluation {
|
||||||
|
decision: Decision::Forbidden,
|
||||||
|
matched_rules: vec![RuleMatch::PrefixRuleMatch {
|
||||||
|
matched_prefix: vec!["rm".to_string()],
|
||||||
|
decision: Decision::Forbidden
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
policy.check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow)
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
Evaluation {
|
||||||
|
decision: Decision::Prompt,
|
||||||
|
matched_rules: vec![RuleMatch::PrefixRuleMatch {
|
||||||
|
matched_prefix: vec!["ls".to_string()],
|
||||||
|
decision: Decision::Prompt
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
policy.check_multiple([vec!["ls".to_string()]].iter(), &|_| Decision::Allow)
|
||||||
|
);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn evaluates_bash_lc_inner_commands() {
|
async fn evaluates_bash_lc_inner_commands() {
|
||||||
let policy_src = r#"
|
let policy_src = r#"
|
||||||
|
|||||||
@@ -230,7 +230,21 @@ fn format_program_name(path: &Path, preserve_program_paths: bool) -> Option<Stri
|
|||||||
|
|
||||||
async fn load_exec_policy() -> anyhow::Result<Policy> {
|
async fn load_exec_policy() -> anyhow::Result<Policy> {
|
||||||
let codex_home = find_codex_home().context("failed to resolve codex_home for execpolicy")?;
|
let codex_home = find_codex_home().context("failed to resolve codex_home for execpolicy")?;
|
||||||
codex_core::load_exec_policy(&codex_home)
|
|
||||||
|
// TODO(mbolin): At a minimum, `cwd` should be configurable via
|
||||||
|
// `codex/sandbox-state/update` or some other custom MCP call.
|
||||||
|
let cwd = None;
|
||||||
|
let cli_overrides = Vec::new();
|
||||||
|
let overrides = codex_core::config_loader::LoaderOverrides::default();
|
||||||
|
let config_layer_stack = codex_core::config_loader::load_config_layers_state(
|
||||||
|
&codex_home,
|
||||||
|
cwd,
|
||||||
|
&cli_overrides,
|
||||||
|
overrides,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
codex_core::load_exec_policy(&config_layer_stack)
|
||||||
.await
|
.await
|
||||||
.map_err(anyhow::Error::from)
|
.map_err(anyhow::Error::from)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -43,6 +43,13 @@ impl AbsolutePathBuf {
|
|||||||
Self::resolve_path_against_base(path, &self.0)
|
Self::resolve_path_against_base(path, &self.0)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn parent(&self) -> Option<Self> {
|
||||||
|
self.0.parent().map(|p| {
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
|
Self::from_absolute_path(p).expect("parent of AbsolutePathBuf must be absolute")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
pub fn as_path(&self) -> &Path {
|
pub fn as_path(&self) -> &Path {
|
||||||
&self.0
|
&self.0
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user