Assemble sandbox/approval/network prompts dynamically (#8961)

- Add a single builder for developer permissions messaging that accepts
SandboxPolicy and approval policy. This builder now drives the developer
“permissions” message that’s injected at session start and any time
sandbox/approval settings change.
- Trim EnvironmentContext to only include cwd, writable roots, and
shell; removed sandbox/approval/network duplication and adjusted XML
serialization and tests accordingly.

Follow-up: adding a config value to replace the developer permissions
message for custom sandboxes.
This commit is contained in:
Ahmed Ibrahim
2026-01-12 15:12:59 -08:00
committed by GitHub
parent 3a6a43ff5c
commit 87f7226cca
30 changed files with 1089 additions and 655 deletions

View File

@@ -1,14 +1,9 @@
use crate::codex::TurnContext;
use crate::protocol::AskForApproval;
use crate::protocol::NetworkAccess;
use crate::protocol::SandboxPolicy;
use crate::shell::Shell;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG;
use codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
use serde::Serialize;
use std::path::PathBuf;
@@ -17,55 +12,12 @@ use std::path::PathBuf;
#[serde(rename = "environment_context", rename_all = "snake_case")]
pub(crate) struct EnvironmentContext {
pub cwd: Option<PathBuf>,
pub approval_policy: Option<AskForApproval>,
pub sandbox_mode: Option<SandboxMode>,
pub network_access: Option<NetworkAccess>,
pub writable_roots: Option<Vec<AbsolutePathBuf>>,
pub shell: Shell,
}
impl EnvironmentContext {
pub fn new(
cwd: Option<PathBuf>,
approval_policy: Option<AskForApproval>,
sandbox_policy: Option<SandboxPolicy>,
shell: Shell,
) -> Self {
Self {
cwd,
approval_policy,
sandbox_mode: match sandbox_policy {
Some(SandboxPolicy::DangerFullAccess) => Some(SandboxMode::DangerFullAccess),
Some(SandboxPolicy::ReadOnly) => Some(SandboxMode::ReadOnly),
Some(SandboxPolicy::ExternalSandbox { .. }) => Some(SandboxMode::DangerFullAccess),
Some(SandboxPolicy::WorkspaceWrite { .. }) => Some(SandboxMode::WorkspaceWrite),
None => None,
},
network_access: match sandbox_policy {
Some(SandboxPolicy::DangerFullAccess) => Some(NetworkAccess::Enabled),
Some(SandboxPolicy::ReadOnly) => Some(NetworkAccess::Restricted),
Some(SandboxPolicy::ExternalSandbox { network_access }) => Some(network_access),
Some(SandboxPolicy::WorkspaceWrite { network_access, .. }) => {
if network_access {
Some(NetworkAccess::Enabled)
} else {
Some(NetworkAccess::Restricted)
}
}
None => None,
},
writable_roots: match sandbox_policy {
Some(SandboxPolicy::WorkspaceWrite { writable_roots, .. }) => {
if writable_roots.is_empty() {
None
} else {
Some(writable_roots)
}
}
_ => None,
},
shell,
}
pub fn new(cwd: Option<PathBuf>, shell: Shell) -> Self {
Self { cwd, shell }
}
/// Compares two environment contexts, ignoring the shell. Useful when
@@ -74,19 +26,11 @@ impl EnvironmentContext {
pub fn equals_except_shell(&self, other: &EnvironmentContext) -> bool {
let EnvironmentContext {
cwd,
approval_policy,
sandbox_mode,
network_access,
writable_roots,
// should compare all fields except shell
shell: _,
} = other;
self.cwd == *cwd
&& self.approval_policy == *approval_policy
&& self.sandbox_mode == *sandbox_mode
&& self.network_access == *network_access
&& self.writable_roots == *writable_roots
}
pub fn diff(before: &TurnContext, after: &TurnContext, shell: &Shell) -> Self {
@@ -95,26 +39,11 @@ impl EnvironmentContext {
} else {
None
};
let approval_policy = if before.approval_policy != after.approval_policy {
Some(after.approval_policy)
} else {
None
};
let sandbox_policy = if before.sandbox_policy != after.sandbox_policy {
Some(after.sandbox_policy.clone())
} else {
None
};
EnvironmentContext::new(cwd, approval_policy, sandbox_policy, shell.clone())
EnvironmentContext::new(cwd, shell.clone())
}
pub fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
Self::new(
Some(turn_context.cwd.clone()),
Some(turn_context.approval_policy),
Some(turn_context.sandbox_policy.clone()),
shell.clone(),
)
Self::new(Some(turn_context.cwd.clone()), shell.clone())
}
}
@@ -126,10 +55,6 @@ impl EnvironmentContext {
/// ```xml
/// <environment_context>
/// <cwd>...</cwd>
/// <approval_policy>...</approval_policy>
/// <sandbox_mode>...</sandbox_mode>
/// <writable_roots>...</writable_roots>
/// <network_access>...</network_access>
/// <shell>...</shell>
/// </environment_context>
/// ```
@@ -138,29 +63,6 @@ impl EnvironmentContext {
if let Some(cwd) = self.cwd {
lines.push(format!(" <cwd>{}</cwd>", cwd.to_string_lossy()));
}
if let Some(approval_policy) = self.approval_policy {
lines.push(format!(
" <approval_policy>{approval_policy}</approval_policy>"
));
}
if let Some(sandbox_mode) = self.sandbox_mode {
lines.push(format!(" <sandbox_mode>{sandbox_mode}</sandbox_mode>"));
}
if let Some(network_access) = self.network_access {
lines.push(format!(
" <network_access>{network_access}</network_access>"
));
}
if let Some(writable_roots) = self.writable_roots {
lines.push(" <writable_roots>".to_string());
for writable_root in writable_roots {
lines.push(format!(
" <root>{}</root>",
writable_root.to_string_lossy()
));
}
lines.push(" </writable_roots>".to_string());
}
let shell_name = self.shell.name();
lines.push(format!(" <shell>{shell_name}</shell>"));
@@ -187,7 +89,6 @@ mod tests {
use super::*;
use core_test_support::test_path_buf;
use core_test_support::test_tmp_path_buf;
use pretty_assertions::assert_eq;
fn fake_shell() -> Shell {
@@ -198,50 +99,17 @@ mod tests {
}
}
fn workspace_write_policy(writable_roots: Vec<&str>, network_access: bool) -> SandboxPolicy {
SandboxPolicy::WorkspaceWrite {
writable_roots: writable_roots
.into_iter()
.map(|s| AbsolutePathBuf::try_from(s).unwrap())
.collect(),
network_access,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
}
}
#[test]
fn serialize_workspace_write_environment_context() {
let cwd = test_path_buf("/repo");
let writable_root = test_tmp_path_buf();
let cwd_str = cwd.to_str().expect("cwd is valid utf-8");
let writable_root_str = writable_root
.to_str()
.expect("writable root is valid utf-8");
let context = EnvironmentContext::new(
Some(cwd.clone()),
Some(AskForApproval::OnRequest),
Some(workspace_write_policy(
vec![cwd_str, writable_root_str],
false,
)),
fake_shell(),
);
let context = EnvironmentContext::new(Some(cwd.clone()), fake_shell());
let expected = format!(
r#"<environment_context>
<cwd>{cwd}</cwd>
<approval_policy>on-request</approval_policy>
<sandbox_mode>workspace-write</sandbox_mode>
<network_access>restricted</network_access>
<writable_roots>
<root>{cwd}</root>
<root>{writable_root}</root>
</writable_roots>
<shell>bash</shell>
</environment_context>"#,
cwd = cwd.display(),
writable_root = writable_root.display(),
);
assert_eq!(context.serialize_to_xml(), expected);
@@ -249,17 +117,9 @@ mod tests {
#[test]
fn serialize_read_only_environment_context() {
let context = EnvironmentContext::new(
None,
Some(AskForApproval::Never),
Some(SandboxPolicy::ReadOnly),
fake_shell(),
);
let context = EnvironmentContext::new(None, fake_shell());
let expected = r#"<environment_context>
<approval_policy>never</approval_policy>
<sandbox_mode>read-only</sandbox_mode>
<network_access>restricted</network_access>
<shell>bash</shell>
</environment_context>"#;
@@ -268,19 +128,9 @@ mod tests {
#[test]
fn serialize_external_sandbox_environment_context() {
let context = EnvironmentContext::new(
None,
Some(AskForApproval::OnRequest),
Some(SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Enabled,
}),
fake_shell(),
);
let context = EnvironmentContext::new(None, fake_shell());
let expected = r#"<environment_context>
<approval_policy>on-request</approval_policy>
<sandbox_mode>danger-full-access</sandbox_mode>
<network_access>enabled</network_access>
<shell>bash</shell>
</environment_context>"#;
@@ -289,19 +139,9 @@ mod tests {
#[test]
fn serialize_external_sandbox_with_restricted_network_environment_context() {
let context = EnvironmentContext::new(
None,
Some(AskForApproval::OnRequest),
Some(SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
}),
fake_shell(),
);
let context = EnvironmentContext::new(None, fake_shell());
let expected = r#"<environment_context>
<approval_policy>on-request</approval_policy>
<sandbox_mode>danger-full-access</sandbox_mode>
<network_access>restricted</network_access>
<shell>bash</shell>
</environment_context>"#;
@@ -310,17 +150,9 @@ mod tests {
#[test]
fn serialize_full_access_environment_context() {
let context = EnvironmentContext::new(
None,
Some(AskForApproval::OnFailure),
Some(SandboxPolicy::DangerFullAccess),
fake_shell(),
);
let context = EnvironmentContext::new(None, fake_shell());
let expected = r#"<environment_context>
<approval_policy>on-failure</approval_policy>
<sandbox_mode>danger-full-access</sandbox_mode>
<network_access>enabled</network_access>
<shell>bash</shell>
</environment_context>"#;
@@ -328,55 +160,24 @@ mod tests {
}
#[test]
fn equals_except_shell_compares_approval_policy() {
// Approval policy
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::OnRequest),
Some(workspace_write_policy(vec!["/repo"], false)),
fake_shell(),
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::Never),
Some(workspace_write_policy(vec!["/repo"], true)),
fake_shell(),
);
assert!(!context1.equals_except_shell(&context2));
fn equals_except_shell_compares_cwd() {
let context1 = EnvironmentContext::new(Some(PathBuf::from("/repo")), fake_shell());
let context2 = EnvironmentContext::new(Some(PathBuf::from("/repo")), fake_shell());
assert!(context1.equals_except_shell(&context2));
}
#[test]
fn equals_except_shell_compares_sandbox_policy() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::OnRequest),
Some(SandboxPolicy::new_read_only_policy()),
fake_shell(),
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::OnRequest),
Some(SandboxPolicy::new_workspace_write_policy()),
fake_shell(),
);
fn equals_except_shell_ignores_sandbox_policy() {
let context1 = EnvironmentContext::new(Some(PathBuf::from("/repo")), fake_shell());
let context2 = EnvironmentContext::new(Some(PathBuf::from("/repo")), fake_shell());
assert!(!context1.equals_except_shell(&context2));
assert!(context1.equals_except_shell(&context2));
}
#[test]
fn equals_except_shell_compares_workspace_write_policy() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::OnRequest),
Some(workspace_write_policy(vec!["/repo", "/tmp", "/var"], false)),
fake_shell(),
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::OnRequest),
Some(workspace_write_policy(vec!["/repo", "/tmp"], true)),
fake_shell(),
);
fn equals_except_shell_compares_cwd_differences() {
let context1 = EnvironmentContext::new(Some(PathBuf::from("/repo1")), fake_shell());
let context2 = EnvironmentContext::new(Some(PathBuf::from("/repo2")), fake_shell());
assert!(!context1.equals_except_shell(&context2));
}
@@ -385,8 +186,6 @@ mod tests {
fn equals_except_shell_ignores_shell() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::OnRequest),
Some(workspace_write_policy(vec!["/repo"], false)),
Shell {
shell_type: ShellType::Bash,
shell_path: "/bin/bash".into(),
@@ -395,8 +194,6 @@ mod tests {
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Some(AskForApproval::OnRequest),
Some(workspace_write_policy(vec!["/repo"], false)),
Shell {
shell_type: ShellType::Zsh,
shell_path: "/bin/zsh".into(),