mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
mutating in memory policy instead of reloading
This commit is contained in:
@@ -866,7 +866,7 @@ impl Session {
|
||||
&self,
|
||||
prefix: &[String],
|
||||
) -> Result<(), ExecPolicyUpdateError> {
|
||||
let (features, codex_home) = {
|
||||
let (features, codex_home, current_policy) = {
|
||||
let state = self.state.lock().await;
|
||||
(
|
||||
state.session_configuration.features.clone(),
|
||||
@@ -875,12 +875,20 @@ impl Session {
|
||||
.original_config_do_not_use
|
||||
.codex_home
|
||||
.clone(),
|
||||
state.session_configuration.exec_policy.clone(),
|
||||
)
|
||||
};
|
||||
|
||||
let policy =
|
||||
crate::exec_policy::append_allow_prefix_rule_and_reload(&features, &codex_home, prefix)
|
||||
.await?;
|
||||
if !features.enabled(Feature::ExecPolicy) {
|
||||
error!("attempted to append execpolicy rule while execpolicy feature is disabled");
|
||||
return Err(ExecPolicyUpdateError::FeatureDisabled);
|
||||
}
|
||||
|
||||
let policy = crate::exec_policy::append_allow_prefix_rule_and_update(
|
||||
&codex_home,
|
||||
current_policy,
|
||||
prefix,
|
||||
)?;
|
||||
|
||||
let mut state = self.state.lock().await;
|
||||
state.session_configuration.exec_policy = policy;
|
||||
@@ -1668,23 +1676,19 @@ mod handlers {
|
||||
decision: ReviewDecision,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
) {
|
||||
if let Some(prefix) = allow_prefix
|
||||
&& matches!(
|
||||
decision,
|
||||
ReviewDecision::Approved
|
||||
| ReviewDecision::ApprovedAllowPrefix
|
||||
| ReviewDecision::ApprovedForSession
|
||||
)
|
||||
&& let Err(err) = sess.persist_command_allow_prefix(&prefix).await
|
||||
{
|
||||
let message = format!("Failed to update execpolicy allow list: {err}");
|
||||
tracing::warn!("{message}");
|
||||
let warning = EventMsg::Warning(WarningEvent { message });
|
||||
sess.send_event_raw(Event {
|
||||
id: id.clone(),
|
||||
msg: warning,
|
||||
})
|
||||
.await;
|
||||
if let Some(prefix) = allow_prefix {
|
||||
if matches!(decision, ReviewDecision::ApprovedAllowPrefix)
|
||||
&& let Err(err) = sess.persist_command_allow_prefix(&prefix).await
|
||||
{
|
||||
let message = format!("Failed to update execpolicy allow list: {err}");
|
||||
tracing::warn!("{message}");
|
||||
let warning = EventMsg::Warning(WarningEvent { message });
|
||||
sess.send_event_raw(Event {
|
||||
id: id.clone(),
|
||||
msg: warning,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
match decision {
|
||||
ReviewDecision::Abort => {
|
||||
|
||||
@@ -6,6 +6,7 @@ use std::sync::Arc;
|
||||
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
|
||||
use codex_execpolicy::AmendError;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Error as ExecPolicyRuleError;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
@@ -53,8 +54,14 @@ pub enum ExecPolicyUpdateError {
|
||||
#[error("failed to update execpolicy file {path}: {source}")]
|
||||
AppendRule { path: PathBuf, source: AmendError },
|
||||
|
||||
#[error("failed to reload execpolicy after updating policy: {0}")]
|
||||
Reload(#[from] ExecPolicyError),
|
||||
#[error("failed to update in-memory execpolicy: {source}")]
|
||||
AddRule {
|
||||
#[from]
|
||||
source: ExecPolicyRuleError,
|
||||
},
|
||||
|
||||
#[error("cannot append execpolicy rule because execpolicy feature is disabled")]
|
||||
FeatureDisabled,
|
||||
}
|
||||
|
||||
pub(crate) async fn exec_policy_for(
|
||||
@@ -100,9 +107,9 @@ pub(crate) fn default_policy_path(codex_home: &Path) -> PathBuf {
|
||||
codex_home.join(POLICY_DIR_NAME).join(DEFAULT_POLICY_FILE)
|
||||
}
|
||||
|
||||
pub(crate) async fn append_allow_prefix_rule_and_reload(
|
||||
features: &Features,
|
||||
pub(crate) fn append_allow_prefix_rule_and_update(
|
||||
codex_home: &Path,
|
||||
mut current_policy: Arc<Policy>,
|
||||
prefix: &[String],
|
||||
) -> Result<Arc<Policy>, ExecPolicyUpdateError> {
|
||||
let policy_path = default_policy_path(codex_home);
|
||||
@@ -113,9 +120,9 @@ pub(crate) async fn append_allow_prefix_rule_and_reload(
|
||||
}
|
||||
})?;
|
||||
|
||||
exec_policy_for(features, codex_home)
|
||||
.await
|
||||
.map_err(ExecPolicyUpdateError::Reload)
|
||||
Arc::make_mut(&mut current_policy).add_prefix_rule(prefix, Decision::Allow)?;
|
||||
|
||||
Ok(current_policy)
|
||||
}
|
||||
|
||||
fn requirement_from_decision(
|
||||
@@ -245,6 +252,7 @@ mod tests {
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::sync::Arc;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[tokio::test]
|
||||
@@ -425,6 +433,50 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_allow_prefix_rule_updates_policy_and_file() {
|
||||
let codex_home = tempdir().expect("create temp dir");
|
||||
let current_policy = Arc::new(Policy::empty());
|
||||
let prefix = vec!["echo".to_string(), "hello".to_string()];
|
||||
|
||||
let updated_policy =
|
||||
append_allow_prefix_rule_and_update(codex_home.path(), current_policy, &prefix)
|
||||
.expect("update policy");
|
||||
|
||||
let evaluation =
|
||||
updated_policy.check(&["echo".to_string(), "hello".to_string(), "world".to_string()]);
|
||||
assert!(matches!(
|
||||
evaluation,
|
||||
Evaluation::Match {
|
||||
decision: Decision::Allow,
|
||||
..
|
||||
}
|
||||
));
|
||||
|
||||
let contents = fs::read_to_string(default_policy_path(codex_home.path()))
|
||||
.expect("policy file should have been created");
|
||||
assert_eq!(
|
||||
contents,
|
||||
"prefix_rule(pattern=[\"echo\", \"hello\"], decision=\"allow\")\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_allow_prefix_rule_rejects_empty_prefix() {
|
||||
let codex_home = tempdir().expect("create temp dir");
|
||||
let current_policy = Arc::new(Policy::empty());
|
||||
|
||||
let result = append_allow_prefix_rule_and_update(codex_home.path(), current_policy, &[]);
|
||||
|
||||
assert!(matches!(
|
||||
result,
|
||||
Err(ExecPolicyUpdateError::AppendRule {
|
||||
source: AmendError::EmptyPrefix,
|
||||
..
|
||||
})
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allow_prefix_is_present_for_single_command_without_policy_match() {
|
||||
let command = vec!["python".to_string()];
|
||||
|
||||
Reference in New Issue
Block a user