Compare commits

...

3 Commits

Author SHA1 Message Date
Dylan Hurd
15a7addd30 fix rebase and compaction 2026-03-02 14:10:14 -07:00
Dylan Hurd
2dd562b2f1 Clarify on-request rule shell feature limits
Document that advanced shell features are excluded from rule evaluation, and stop listing subshell boundaries as independent command segments.

Co-authored-by: Codex <noreply@openai.com>
2026-03-02 13:43:54 -07:00
Dylan Hurd
3eba5057f8 fix(core) Move approved rules to env context 2026-03-02 13:43:54 -07:00
14 changed files with 192 additions and 29 deletions

View File

@@ -625,6 +625,7 @@ fn append_rollout_turn_context(path: &Path, timestamp: &str, model: &str) -> std
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
network: None,
approved_prefix_rules: None,
model: model.to_string(),
personality: None,
collaboration_mode: None,

View File

@@ -155,6 +155,7 @@ use crate::error::Result as CodexResult;
#[cfg(test)]
use crate::exec::StreamOutput;
use codex_config::CONFIG_TOML_FILE;
use codex_execpolicy::Policy;
mod rollout_reconstruction;
#[cfg(test)]
@@ -770,7 +771,24 @@ impl TurnContext {
.unwrap_or(compact::SUMMARIZATION_PROMPT)
}
#[cfg(test)]
pub(crate) fn to_turn_context_item(&self) -> TurnContextItem {
self.to_turn_context_item_with_approved_prefix_rules(None)
}
pub(crate) fn to_turn_context_item_with_exec_policy(
&self,
exec_policy: &Policy,
) -> TurnContextItem {
self.to_turn_context_item_with_approved_prefix_rules(format_allow_prefixes(
exec_policy.get_allowed_prefixes(),
))
}
fn to_turn_context_item_with_approved_prefix_rules(
&self,
approved_prefix_rules: Option<String>,
) -> TurnContextItem {
TurnContextItem {
turn_id: Some(self.sub_id.clone()),
cwd: self.cwd.clone(),
@@ -779,6 +797,7 @@ impl TurnContext {
approval_policy: self.approval_policy.value(),
sandbox_policy: self.sandbox_policy.get().clone(),
network: self.turn_context_network_item(),
approved_prefix_rules,
model: self.model_info.slug.clone(),
personality: self.personality,
collaboration_mode: Some(self.collaboration_mode.clone()),
@@ -1787,7 +1806,11 @@ impl Session {
self.record_conversation_items(&turn_context, &items).await;
{
let mut state = self.state.lock().await;
state.set_reference_context_item(Some(turn_context.to_turn_context_item()));
state.set_reference_context_item(Some(
turn_context.to_turn_context_item_with_exec_policy(
self.services.exec_policy.current().as_ref(),
),
));
}
self.set_previous_turn_settings(None).await;
// Ensure initial items are visible to immediate readers (e.g., tests, forks).
@@ -1900,7 +1923,11 @@ impl Session {
.await;
{
let mut state = self.state.lock().await;
state.set_reference_context_item(Some(turn_context.to_turn_context_item()));
state.set_reference_context_item(Some(
turn_context.to_turn_context_item_with_exec_policy(
self.services.exec_policy.current().as_ref(),
),
));
}
// Forked threads should remain file-backed immediately after startup.
@@ -3087,9 +3114,13 @@ impl Session {
.format_environment_context_subagents(self.conversation_id)
.await;
contextual_user_sections.push(
EnvironmentContext::from_turn_context(turn_context, shell.as_ref())
.with_subagents(subagents)
.serialize_to_xml(),
EnvironmentContext::from_turn_context(
turn_context,
shell.as_ref(),
format_allow_prefixes(self.services.exec_policy.current().get_allowed_prefixes()),
)
.with_subagents(subagents)
.serialize_to_xml(),
);
let mut items = Vec::with_capacity(2);
@@ -3157,7 +3188,8 @@ impl Session {
self.build_settings_update_items(reference_context_item.as_ref(), turn_context)
.await
};
let turn_context_item = turn_context.to_turn_context_item();
let turn_context_item = turn_context
.to_turn_context_item_with_exec_policy(self.services.exec_policy.current().as_ref());
if !context_items.is_empty() {
self.record_conversation_items(turn_context, &context_items)
.await;
@@ -7463,6 +7495,7 @@ mod tests {
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),

View File

@@ -46,6 +46,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
@@ -84,6 +85,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
@@ -745,6 +747,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
@@ -816,6 +819,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
@@ -844,6 +848,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
@@ -949,6 +954,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: current_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
@@ -1050,6 +1056,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
@@ -1193,6 +1200,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: turn_context.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: turn_context.personality,
collaboration_mode: Some(turn_context.collaboration_mode.clone()),

View File

@@ -211,7 +211,11 @@ async fn run_compact_task_inner(
new_history.extend(ghost_snapshots);
let reference_context_item = match initial_context_injection {
InitialContextInjection::DoNotInject => None,
InitialContextInjection::BeforeLastUserMessage => Some(turn_context.to_turn_context_item()),
InitialContextInjection::BeforeLastUserMessage => {
Some(turn_context.to_turn_context_item_with_exec_policy(
sess.services.exec_policy.current().as_ref(),
))
}
};
let compacted_item = CompactedItem {
message: summary_text.clone(),

View File

@@ -135,7 +135,11 @@ async fn run_remote_compact_task_inner_impl(
}
let reference_context_item = match initial_context_injection {
InitialContextInjection::DoNotInject => None,
InitialContextInjection::BeforeLastUserMessage => Some(turn_context.to_turn_context_item()),
InitialContextInjection::BeforeLastUserMessage => {
Some(turn_context.to_turn_context_item_with_exec_policy(
sess.services.exec_policy.current().as_ref(),
))
}
};
let compacted_item = CompactedItem {
message: String::new(),

View File

@@ -8,6 +8,7 @@ use codex_protocol::config_types::Personality;
use codex_protocol::models::ContentItem;
use codex_protocol::models::DeveloperInstructions;
use codex_protocol::models::ResponseItem;
use codex_protocol::models::format_allow_prefixes;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::protocol::TurnContextItem;
@@ -15,16 +16,19 @@ fn build_environment_update_item(
previous: Option<&TurnContextItem>,
next: &TurnContext,
shell: &Shell,
exec_policy: &Policy,
) -> Option<ResponseItem> {
let prev = previous?;
let prev_context = EnvironmentContext::from_turn_context_item(prev, shell);
let next_context = EnvironmentContext::from_turn_context(next, shell);
let approved_prefix_rules = format_allow_prefixes(exec_policy.get_allowed_prefixes());
let next_context =
EnvironmentContext::from_turn_context(next, shell, approved_prefix_rules.clone());
if prev_context.equals_except_shell(&next_context) {
return None;
}
Some(ResponseItem::from(
EnvironmentContext::diff_from_turn_context_item(prev, next, shell),
EnvironmentContext::diff_from_turn_context_item(prev, next, shell, approved_prefix_rules),
))
}
@@ -181,7 +185,7 @@ pub(crate) fn build_settings_update_items(
exec_policy: &Policy,
personality_feature_enabled: bool,
) -> Vec<ResponseItem> {
let contextual_user_message = build_environment_update_item(previous, next, shell);
let contextual_user_message = build_environment_update_item(previous, next, shell, exec_policy);
let developer_update_sections = [
// Keep model-switch instructions first so model-specific guidance is read before
// any other context diffs on this turn.

View File

@@ -16,6 +16,7 @@ pub(crate) struct EnvironmentContext {
pub current_date: Option<String>,
pub timezone: Option<String>,
pub network: Option<NetworkContext>,
pub approved_prefix_rules: Option<String>,
pub subagents: Option<String>,
}
@@ -32,6 +33,7 @@ impl EnvironmentContext {
current_date: Option<String>,
timezone: Option<String>,
network: Option<NetworkContext>,
approved_prefix_rules: Option<String>,
subagents: Option<String>,
) -> Self {
Self {
@@ -40,6 +42,7 @@ impl EnvironmentContext {
current_date,
timezone,
network,
approved_prefix_rules,
subagents,
}
}
@@ -53,6 +56,7 @@ impl EnvironmentContext {
current_date,
timezone,
network,
approved_prefix_rules,
subagents,
shell: _,
} = other;
@@ -60,6 +64,7 @@ impl EnvironmentContext {
&& self.current_date == *current_date
&& self.timezone == *timezone
&& self.network == *network
&& self.approved_prefix_rules == *approved_prefix_rules
&& self.subagents == *subagents
}
@@ -67,9 +72,11 @@ impl EnvironmentContext {
before: &TurnContextItem,
after: &TurnContext,
shell: &Shell,
approved_prefix_rules: Option<String>,
) -> Self {
let before_network = Self::network_from_turn_context_item(before);
let after_network = Self::network_from_turn_context(after);
let before_approved_prefix_rules = before.approved_prefix_rules.clone();
let cwd = if before.cwd != after.cwd {
Some(after.cwd.clone())
} else {
@@ -82,16 +89,34 @@ impl EnvironmentContext {
} else {
before_network
};
EnvironmentContext::new(cwd, shell.clone(), current_date, timezone, network, None)
let approved_prefix_rules = if before_approved_prefix_rules != approved_prefix_rules {
approved_prefix_rules
} else {
before_approved_prefix_rules
};
EnvironmentContext::new(
cwd,
shell.clone(),
current_date,
timezone,
network,
approved_prefix_rules,
None,
)
}
pub fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
pub fn from_turn_context(
turn_context: &TurnContext,
shell: &Shell,
approved_prefix_rules: Option<String>,
) -> Self {
Self::new(
Some(turn_context.cwd.clone()),
shell.clone(),
turn_context.current_date.clone(),
turn_context.timezone.clone(),
Self::network_from_turn_context(turn_context),
approved_prefix_rules,
None,
)
}
@@ -103,6 +128,7 @@ impl EnvironmentContext {
turn_context_item.current_date.clone(),
turn_context_item.timezone.clone(),
Self::network_from_turn_context_item(turn_context_item),
turn_context_item.approved_prefix_rules.clone(),
None,
)
}
@@ -183,6 +209,13 @@ impl EnvironmentContext {
// lines.push(" <network enabled=\"false\" />".to_string());
}
}
if let Some(approved_prefix_rules) = self.approved_prefix_rules {
lines.push(" <approved_prefix_rules>".to_string());
for line in approved_prefix_rules.lines() {
lines.push(format!(" {line}"));
}
lines.push(" </approved_prefix_rules>".to_string());
}
if let Some(subagents) = self.subagents {
lines.push(" <subagents>".to_string());
lines.extend(subagents.lines().map(|line| format!(" {line}")));
@@ -224,6 +257,7 @@ mod tests {
Some("America/Los_Angeles".to_string()),
None,
None,
None,
);
let expected = format!(
@@ -252,6 +286,7 @@ mod tests {
Some("America/Los_Angeles".to_string()),
Some(network),
None,
None,
);
let expected = format!(
@@ -281,6 +316,7 @@ mod tests {
Some("America/Los_Angeles".to_string()),
None,
None,
None,
);
let expected = r#"<environment_context>
@@ -301,6 +337,7 @@ mod tests {
Some("America/Los_Angeles".to_string()),
None,
None,
None,
);
let expected = r#"<environment_context>
@@ -321,6 +358,7 @@ mod tests {
Some("America/Los_Angeles".to_string()),
None,
None,
None,
);
let expected = r#"<environment_context>
@@ -341,6 +379,7 @@ mod tests {
Some("America/Los_Angeles".to_string()),
None,
None,
None,
);
let expected = r#"<environment_context>
@@ -352,6 +391,35 @@ mod tests {
assert_eq!(context.serialize_to_xml(), expected);
}
#[test]
fn serialize_environment_context_with_approved_prefix_rules() {
let context = EnvironmentContext::new(
Some(test_path_buf("/repo")),
fake_shell(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
None,
Some("- [\"mkdir\"]\n- [\"gh\", \"api\"]".to_string()),
None,
);
let expected = format!(
r#"<environment_context>
<cwd>{}</cwd>
<shell>bash</shell>
<current_date>2026-02-26</current_date>
<timezone>America/Los_Angeles</timezone>
<approved_prefix_rules>
- ["mkdir"]
- ["gh", "api"]
</approved_prefix_rules>
</environment_context>"#,
test_path_buf("/repo").display()
);
assert_eq!(context.serialize_to_xml(), expected);
}
#[test]
fn equals_except_shell_compares_cwd() {
let context1 = EnvironmentContext::new(
@@ -361,6 +429,7 @@ mod tests {
None,
None,
None,
None,
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
@@ -369,6 +438,7 @@ mod tests {
None,
None,
None,
None,
);
assert!(context1.equals_except_shell(&context2));
}
@@ -382,6 +452,7 @@ mod tests {
None,
None,
None,
None,
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
@@ -390,6 +461,7 @@ mod tests {
None,
None,
None,
None,
);
assert!(context1.equals_except_shell(&context2));
@@ -404,6 +476,7 @@ mod tests {
None,
None,
None,
None,
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo2")),
@@ -412,6 +485,7 @@ mod tests {
None,
None,
None,
None,
);
assert!(!context1.equals_except_shell(&context2));
@@ -430,6 +504,7 @@ mod tests {
None,
None,
None,
None,
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
@@ -442,6 +517,7 @@ mod tests {
None,
None,
None,
None,
);
assert!(context1.equals_except_shell(&context2));
@@ -455,6 +531,7 @@ mod tests {
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
None,
None,
Some("- agent-1: atlas\n- agent-2".to_string()),
);
@@ -474,4 +551,28 @@ mod tests {
assert_eq!(context.serialize_to_xml(), expected);
}
#[test]
fn equals_except_shell_compares_approved_prefix_rules() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
fake_shell(),
None,
None,
None,
Some("- [\"mkdir\"]".to_string()),
None,
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
fake_shell(),
None,
None,
None,
Some("- [\"gh\", \"api\"]".to_string()),
None,
);
assert!(!context1.equals_except_shell(&context2));
}
}

View File

@@ -1401,6 +1401,7 @@ mod tests {
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
network: None,
approved_prefix_rules: None,
model: "test-model".to_string(),
personality: None,
collaboration_mode: None,

View File

@@ -33,6 +33,7 @@ fn resume_history(
approval_policy: config.permissions.approval_policy.value(),
sandbox_policy: config.permissions.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model: previous_model.to_string(),
personality: None,
collaboration_mode: None,

View File

@@ -352,7 +352,7 @@ impl DeveloperInstructions {
pub fn from(
approval_policy: AskForApproval,
exec_policy: &Policy,
_exec_policy: &Policy,
request_permission_enabled: bool,
) -> DeveloperInstructions {
let on_request_instructions = || {
@@ -361,15 +361,7 @@ impl DeveloperInstructions {
} else {
APPROVAL_POLICY_ON_REQUEST_RULE
};
let command_prefixes = format_allow_prefixes(exec_policy.get_allowed_prefixes());
match command_prefixes {
Some(prefixes) => {
format!(
"{on_request_rule}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}"
)
}
None => on_request_rule.to_string(),
}
on_request_rule.to_string()
};
let text = match approval_policy {
AskForApproval::Never => APPROVAL_POLICY_NEVER.to_string(),
@@ -533,7 +525,10 @@ impl DeveloperInstructions {
SandboxMode::WorkspaceWrite => SANDBOX_MODE_WORKSPACE_WRITE.trim_end(),
SandboxMode::ReadOnly => SANDBOX_MODE_READ_ONLY.trim_end(),
};
let text = template.replace("{network_access}", &network_access.to_string());
let text = format!(
"{} Approved command prefix rules (if any) are provided in `<environment_context>` under `<approved_prefix_rules>`.",
template.replace("{network_access}", &network_access.to_string())
);
DeveloperInstructions::new(text)
}
@@ -575,6 +570,10 @@ pub fn format_allow_prefixes(prefixes: Vec<Vec<String>>) -> Option<String> {
output = output[..byte_idx].to_string();
}
if output.is_empty() {
return None;
}
if truncated {
Some(format!("{output}{TRUNCATED_MARKER}"))
} else {
@@ -1331,7 +1330,7 @@ mod tests {
assert_eq!(
workspace_write,
DeveloperInstructions::new(
"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."
"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. Approved command prefix rules (if any) are provided in `<environment_context>` under `<approved_prefix_rules>`."
)
);
@@ -1339,7 +1338,7 @@ mod tests {
assert_eq!(
read_only,
DeveloperInstructions::new(
"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."
"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. Approved command prefix rules (if any) are provided in `<environment_context>` under `<approved_prefix_rules>`."
)
);
}
@@ -1408,8 +1407,8 @@ mod tests {
let text = instructions.into_text();
assert!(text.contains("prefix_rule"));
assert!(text.contains("Approved command prefixes"));
assert!(text.contains(r#"["git", "pull"]"#));
assert!(text.contains("<approved_prefix_rules>"));
assert!(!text.contains("Approved command prefixes"));
}
#[test]

View File

@@ -5,7 +5,6 @@ Commands are run outside the sandbox if they are approved by the user, or match
- Pipes: |
- Logical operators: &&, ||
- Command separators: ;
- Subshell boundaries: (...), $(...)
Each resulting segment is evaluated independently for sandbox restrictions and approval requirements.
@@ -19,6 +18,8 @@ This is treated as two command segments:
["tee", "output.txt"]
Commands that use more advanced shell features like redirection (>, >>, <), substitutions ($(...), ...), environment variables (FOO=bar), or wildcard patterns (*, ?) will not be evaluated against rules, to limit the scope of what an approved rule allows.
## How to request escalation
IMPORTANT: To request approval to execute a command that will require escalated privileges:

View File

@@ -2146,6 +2146,8 @@ pub struct TurnContextItem {
pub sandbox_policy: SandboxPolicy,
#[serde(skip_serializing_if = "Option::is_none")]
pub network: Option<TurnContextNetworkItem>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub approved_prefix_rules: Option<String>,
pub model: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub personality: Option<Personality>,
@@ -3380,6 +3382,7 @@ mod tests {
allowed_domains: vec!["api.example.com".to_string()],
denied_domains: vec!["blocked.example.com".to_string()],
}),
approved_prefix_rules: None,
model: "gpt-5".to_string(),
personality: None,
collaboration_mode: None,

View File

@@ -258,6 +258,7 @@ mod tests {
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
network: None,
approved_prefix_rules: None,
model: "gpt-5".to_string(),
personality: None,
collaboration_mode: None,
@@ -295,6 +296,7 @@ mod tests {
approval_policy: AskForApproval::OnRequest,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
network: None,
approved_prefix_rules: None,
model: "gpt-5".to_string(),
personality: None,
collaboration_mode: None,

View File

@@ -1277,6 +1277,7 @@ mod tests {
approval_policy: config.permissions.approval_policy.value(),
sandbox_policy: config.permissions.sandbox_policy.get().clone(),
network: None,
approved_prefix_rules: None,
model,
personality: None,
collaboration_mode: None,