mirror of
https://github.com/openai/codex.git
synced 2026-09-03 14:59:03 +00:00
Preserve parent sandbox enforcement for memory consolidation (#32441)
## What changed - Pass the parent turn's effective permission profile to the memory consolidation agent, including thread-level permission and legacy sandbox overrides. - Preserve disabled and externally enforced permission profiles instead of replacing them with a managed sandbox. - Continue restricting consolidation to the memory root without network access when the parent uses Codex-managed permissions. ## Testing - Add coverage for disabled, external, and managed parent permission profiles. GitOrigin-RevId: 9ca3be0e41dc14d858053f6f70e33f4ae7c578e1
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
use super::*;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::FunctionCallOutputContentItem;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AdditionalContextEntry as CoreAdditionalContextEntry;
|
||||
use codex_protocol::protocol::AdditionalContextKind as CoreAdditionalContextKind;
|
||||
use codex_protocol::protocol::MultiAgentVersion;
|
||||
@@ -517,6 +518,13 @@ impl TurnRequestProcessor {
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
let parent_permission_profile_override =
|
||||
thread_settings.permission_profile.clone().or_else(|| {
|
||||
thread_settings
|
||||
.sandbox_policy
|
||||
.as_ref()
|
||||
.map(PermissionProfile::from_legacy_sandbox_policy)
|
||||
});
|
||||
|
||||
// Start the turn by submitting the user input. Return its submission id as turn_id.
|
||||
let turn_op = Op::UserInput {
|
||||
@@ -541,12 +549,15 @@ impl TurnRequestProcessor {
|
||||
|
||||
if turn_has_input {
|
||||
let config_snapshot = thread.config_snapshot().await;
|
||||
let parent_permission_profile =
|
||||
parent_permission_profile_override.unwrap_or(config_snapshot.permission_profile);
|
||||
codex_memories_write::start_memories_startup_task(
|
||||
Arc::clone(&self.thread_manager),
|
||||
Arc::clone(&self.auth_manager),
|
||||
thread_id,
|
||||
Arc::clone(&thread),
|
||||
thread.config().await,
|
||||
parent_permission_profile,
|
||||
&config_snapshot.session_source,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ use codex_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
use codex_model_provider::ModelProvider;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AgentStatus;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
@@ -44,7 +45,11 @@ struct Counters {
|
||||
|
||||
/// Runs memory phase 2 (aka consolidation) in strict order. The method represents the linear
|
||||
/// flow of the consolidation phase.
|
||||
pub async fn run(context: Arc<MemoryStartupContext>, config: Arc<Config>) {
|
||||
pub async fn run(
|
||||
context: Arc<MemoryStartupContext>,
|
||||
config: Arc<Config>,
|
||||
parent_permission_profile: PermissionProfile,
|
||||
) {
|
||||
let phase_two_e2e_timer = context.start_timer(MEMORY_PHASE_TWO_E2E_MS);
|
||||
|
||||
let Some(db) = context.state_db() else {
|
||||
@@ -78,7 +83,11 @@ pub async fn run(context: Arc<MemoryStartupContext>, config: Arc<Config>) {
|
||||
}
|
||||
|
||||
// 3. Build the locked-down config used by the consolidation agent.
|
||||
let Some(agent_config) = agent::get_config(config.as_ref(), context.provider()) else {
|
||||
let Some(agent_config) = agent::get_config(
|
||||
config.as_ref(),
|
||||
parent_permission_profile,
|
||||
context.provider(),
|
||||
) else {
|
||||
// If we can't get the config, we can't consolidate.
|
||||
tracing::error!("failed to get agent config");
|
||||
job::failed(
|
||||
@@ -299,7 +308,11 @@ mod agent {
|
||||
use super::*;
|
||||
use tracing::warn;
|
||||
|
||||
pub(super) fn get_config(config: &Config, provider: &dyn ModelProvider) -> Option<Config> {
|
||||
pub(super) fn get_config(
|
||||
config: &Config,
|
||||
parent_permission_profile: PermissionProfile,
|
||||
provider: &dyn ModelProvider,
|
||||
) -> Option<Config> {
|
||||
let root = memory_root(&config.codex_home);
|
||||
let mut agent_config = config.clone();
|
||||
|
||||
@@ -322,18 +335,25 @@ mod agent {
|
||||
.features
|
||||
.disable(Feature::SkillMcpDependencyInstall);
|
||||
|
||||
// Sandbox policy
|
||||
let writable_roots = vec![root];
|
||||
// The consolidation agent only needs local memory-root write access and no network.
|
||||
let consolidation_sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
agent_config
|
||||
.set_legacy_sandbox_policy(consolidation_sandbox_policy)
|
||||
.ok()?;
|
||||
// Preserve the parent's explicit choice to skip Codex-managed sandboxing.
|
||||
match parent_permission_profile {
|
||||
PermissionProfile::Disabled => agent_config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::Disabled),
|
||||
PermissionProfile::External { network } => agent_config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::External { network }),
|
||||
PermissionProfile::Managed { .. } => {
|
||||
// The consolidation agent only needs local memory-root write access and no network.
|
||||
agent_config.set_legacy_sandbox_policy(SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![root],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
})
|
||||
}
|
||||
}
|
||||
.ok()?;
|
||||
|
||||
agent_config.model = Some(
|
||||
config
|
||||
@@ -533,6 +553,9 @@ mod agent {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "phase2_sandbox_tests.rs"]
|
||||
mod sandbox_tests;
|
||||
#[cfg(test)]
|
||||
#[path = "phase2_workspace_roots_tests.rs"]
|
||||
mod workspace_roots_tests;
|
||||
|
||||
67
codex-rs/memories/write/src/phase2_sandbox_tests.rs
Normal file
67
codex-rs/memories/write/src/phase2_sandbox_tests.rs
Normal file
@@ -0,0 +1,67 @@
|
||||
use super::agent;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_protocol::models::ManagedFileSystemPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[tokio::test]
|
||||
async fn consolidation_uses_canonical_parent_enforcement() -> anyhow::Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
let test = test_codex()
|
||||
.with_home(home)
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
let provider = create_model_provider(
|
||||
test.config.model_provider.clone(),
|
||||
Some(test.thread_manager.auth_manager()),
|
||||
);
|
||||
|
||||
let root = crate::memory_root(&test.config.codex_home);
|
||||
let managed_worker_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![root.clone()],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
for (parent_permission_profile, expected_permission_profile) in [
|
||||
(PermissionProfile::Disabled, PermissionProfile::Disabled),
|
||||
(
|
||||
PermissionProfile::External {
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
},
|
||||
PermissionProfile::External {
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
},
|
||||
),
|
||||
(
|
||||
PermissionProfile::Managed {
|
||||
file_system: ManagedFileSystemPermissions::Unrestricted,
|
||||
network: NetworkSandboxPolicy::Enabled,
|
||||
},
|
||||
PermissionProfile::from_legacy_sandbox_policy_for_cwd(
|
||||
&managed_worker_policy,
|
||||
root.as_path(),
|
||||
),
|
||||
),
|
||||
] {
|
||||
let agent_config =
|
||||
agent::get_config(&test.config, parent_permission_profile, provider.as_ref())
|
||||
.expect("agent config should be created");
|
||||
|
||||
assert_eq!(
|
||||
agent_config.permissions.permission_profile(),
|
||||
&expected_permission_profile
|
||||
);
|
||||
}
|
||||
|
||||
test.codex.shutdown_and_wait().await?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -21,8 +21,10 @@ async fn consolidation_rebinds_workspace_roots_to_memory_root() -> anyhow::Resul
|
||||
Some(test.thread_manager.auth_manager()),
|
||||
);
|
||||
|
||||
let parent_permission_profile = test.config.permissions.effective_permission_profile();
|
||||
let agent_config =
|
||||
agent::get_config(&test.config, provider.as_ref()).expect("agent config should be created");
|
||||
agent::get_config(&test.config, parent_permission_profile, provider.as_ref())
|
||||
.expect("agent config should be created");
|
||||
let root = memory_root(&test.config.codex_home);
|
||||
|
||||
assert_eq!(agent_config.cwd, root);
|
||||
|
||||
@@ -11,6 +11,7 @@ use codex_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
use codex_login::AuthManager;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use std::sync::Arc;
|
||||
use tracing::warn;
|
||||
@@ -25,6 +26,7 @@ pub fn start_memories_startup_task(
|
||||
thread_id: ThreadId,
|
||||
thread: Arc<CodexThread>,
|
||||
config: Arc<Config>,
|
||||
parent_permission_profile: PermissionProfile,
|
||||
source: &SessionSource,
|
||||
) {
|
||||
if config.ephemeral
|
||||
@@ -74,6 +76,6 @@ pub fn start_memories_startup_task(
|
||||
// Run phase 1.
|
||||
phase1::run(Arc::clone(&context), Arc::clone(&config)).await;
|
||||
// Run phase 2.
|
||||
phase2::run(context, config).await;
|
||||
phase2::run(context, config, parent_permission_profile).await;
|
||||
});
|
||||
}
|
||||
|
||||
@@ -554,7 +554,8 @@ async fn run_memory_phase_two_model_request_test(
|
||||
tokio::fs::create_dir_all(&root).await?;
|
||||
seed_extension_instructions(&root).await?;
|
||||
seed_required_memory_artifacts(&root).await?;
|
||||
phase2::run(context, config).await;
|
||||
let parent_permission_profile = config.permissions.effective_permission_profile();
|
||||
phase2::run(context, config, parent_permission_profile).await;
|
||||
let request = wait_for_single_request(&response).await;
|
||||
wait_for_phase2_workspace_reset(&home.path().join("memories")).await?;
|
||||
shutdown_test_codex(&test).await?;
|
||||
@@ -608,12 +609,14 @@ async fn trigger_memories_startup(test: &TestCodex) {
|
||||
.features
|
||||
.enable(Feature::MemoryTool)
|
||||
.expect("test config should allow feature update");
|
||||
let parent_permission_profile = config.permissions.effective_permission_profile();
|
||||
start_memories_startup_task(
|
||||
Arc::clone(&test.thread_manager),
|
||||
test.thread_manager.auth_manager(),
|
||||
test.session_configured.thread_id,
|
||||
Arc::clone(&test.codex),
|
||||
Arc::new(config),
|
||||
parent_permission_profile,
|
||||
&config_snapshot.session_source,
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user