mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Support execution-host context when resolving cloud config (#38086)
## What changed - Add a scoped `AbsolutePathBufGuard::with_home_directory` override so `~` paths can resolve against an explicitly supplied home directory while preserving the existing base-directory behavior. Nested overrides restore the previous home directory. - Expose `compose_requirements_for_hostname` so callers can evaluate `remote_sandbox_config` for a supplied execution-host hostname. - Cover cloud bundle conversion with execution-host path expansion and hostname-specific sandbox requirements. GitOrigin-RevId: 6fd486992b45ff92e22bb4073b888252b544f285
This commit is contained in:
@@ -1,7 +1,15 @@
|
||||
use super::*;
|
||||
use crate::AbsolutePathBufGuard;
|
||||
use crate::ConfigLayerSource;
|
||||
use crate::ConfigRequirementsToml;
|
||||
use crate::FilesystemDenyReadPattern;
|
||||
use crate::SandboxModeRequirement;
|
||||
use crate::compose_requirements;
|
||||
use crate::compose_requirements_for_hostname;
|
||||
use crate::config_requirements::FilesystemRequirementsToml;
|
||||
use crate::config_requirements::PermissionsRequirementsToml;
|
||||
use crate::config_toml::ConfigToml;
|
||||
use crate::types::SandboxWorkspaceWrite;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
@@ -163,3 +171,89 @@ fn bundle_layers_can_strict_validate_enterprise_managed_config() {
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bundle_layers_resolve_paths_and_requirements_for_the_execution_host() {
|
||||
let temp_dir = tempdir().expect("temporary directories");
|
||||
let executor_home = temp_dir.path().join("executor-home");
|
||||
let executor_codex_home = AbsolutePathBuf::from_absolute_path(executor_home.join(".codex"))
|
||||
.expect("absolute executor Codex home");
|
||||
let bundle = CloudConfigBundle {
|
||||
config_toml: CloudConfigTomlBundle {
|
||||
enterprise_managed: vec![CloudConfigFragment {
|
||||
id: "config".to_string(),
|
||||
name: "Executor config".to_string(),
|
||||
contents: r#"
|
||||
[sandbox_workspace_write]
|
||||
writable_roots = ["~/cloud-root", "./relative-root"]
|
||||
"#
|
||||
.to_string(),
|
||||
}],
|
||||
},
|
||||
requirements_toml: CloudRequirementsTomlBundle {
|
||||
enterprise_managed: vec![CloudRequirementsFragment {
|
||||
id: "requirements".to_string(),
|
||||
name: "Executor requirements".to_string(),
|
||||
contents: r#"
|
||||
[permissions.filesystem]
|
||||
deny_read = ["~/private"]
|
||||
|
||||
[[remote_sandbox_config]]
|
||||
hostname_patterns = ["executor-*"]
|
||||
allowed_sandbox_modes = ["read-only"]
|
||||
"#
|
||||
.to_string(),
|
||||
}],
|
||||
},
|
||||
};
|
||||
|
||||
let (config, requirements) = AbsolutePathBufGuard::with_home_directory(&executor_home, || {
|
||||
let layers =
|
||||
CloudConfigBundleLayers::from_bundle_strict_config(bundle, &executor_codex_home)
|
||||
.expect("executor bundle should convert into layers");
|
||||
let config: ConfigToml = layers.enterprise_managed_config[0]
|
||||
.config
|
||||
.clone()
|
||||
.try_into()
|
||||
.expect("deserialize executor config");
|
||||
let requirements = compose_requirements_for_hostname(
|
||||
layers.enterprise_managed_requirements,
|
||||
Some("executor-01"),
|
||||
)
|
||||
.expect("compose executor requirements")
|
||||
.expect("executor requirements should be present")
|
||||
.into_toml();
|
||||
(config, requirements)
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
config.sandbox_workspace_write,
|
||||
Some(SandboxWorkspaceWrite {
|
||||
writable_roots: vec![
|
||||
AbsolutePathBuf::from_absolute_path(executor_home.join("cloud-root"))
|
||||
.expect("absolute cloud root"),
|
||||
AbsolutePathBuf::from_absolute_path(
|
||||
executor_codex_home.as_path().join("relative-root"),
|
||||
)
|
||||
.expect("absolute relative root"),
|
||||
],
|
||||
..Default::default()
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
requirements,
|
||||
ConfigRequirementsToml {
|
||||
allowed_sandbox_modes: Some(vec![SandboxModeRequirement::ReadOnly]),
|
||||
permissions: Some(PermissionsRequirementsToml {
|
||||
filesystem: Some(FilesystemRequirementsToml {
|
||||
deny_read: Some(vec![FilesystemDenyReadPattern::from(
|
||||
AbsolutePathBuf::from_absolute_path(executor_home.join("private"))
|
||||
.expect("absolute private root"),
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -52,6 +52,7 @@ pub use codex_protocol::config_types::ProfileV2Name;
|
||||
pub use codex_protocol::config_types::ProfileV2NameParseError;
|
||||
pub use codex_protocol::config_types::ToolExposureSurface;
|
||||
pub use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
pub use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
pub use config_layer_source::ConfigLayer;
|
||||
pub use config_layer_source::ConfigLayerMetadata;
|
||||
pub use config_layer_source::ConfigLayerSource;
|
||||
@@ -151,6 +152,7 @@ pub use requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml;
|
||||
pub use requirements_exec_policy::RequirementsExecPolicyToml;
|
||||
pub use requirements_layers::RequirementsLayerEntry;
|
||||
pub use requirements_layers::compose_requirements;
|
||||
pub use requirements_layers::compose_requirements_for_hostname;
|
||||
pub use shell_environment_policy::validate_shell_environment_policy_filter_config;
|
||||
pub use skills_config::BundledSkillsConfig;
|
||||
pub use skills_config::SkillConfig;
|
||||
|
||||
@@ -7,3 +7,4 @@ mod stack;
|
||||
|
||||
pub use layer::RequirementsLayerEntry;
|
||||
pub use stack::compose_requirements;
|
||||
pub use stack::compose_requirements_for_hostname;
|
||||
|
||||
@@ -63,8 +63,8 @@ pub fn compose_requirements(
|
||||
compose_requirements_with_hostname_resolver(layers, crate::host_name)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(super) fn compose_requirements_for_hostname(
|
||||
/// Composes requirements using an explicitly supplied execution-host hostname.
|
||||
pub fn compose_requirements_for_hostname(
|
||||
layers: impl IntoIterator<Item = RequirementsLayerEntry>,
|
||||
hostname: Option<&str>,
|
||||
) -> Result<Option<ConfigRequirementsWithSources>, RequirementsCompositionError> {
|
||||
|
||||
@@ -26,8 +26,10 @@ pub struct AbsolutePathBuf(PathBuf);
|
||||
impl AbsolutePathBuf {
|
||||
fn maybe_expand_home_directory(path: &Path) -> PathBuf {
|
||||
if let Some(path_str) = path.to_str()
|
||||
&& let Some(home) = home_dir()
|
||||
&& let Some(rest) = path_str.strip_prefix('~')
|
||||
&& let Some(home) = ABSOLUTE_PATH_HOME
|
||||
.with(|cell| cell.borrow().clone())
|
||||
.or_else(home_dir)
|
||||
{
|
||||
if rest.is_empty() {
|
||||
return home;
|
||||
@@ -326,6 +328,7 @@ impl TryFrom<String> for AbsolutePathBuf {
|
||||
|
||||
thread_local! {
|
||||
static ABSOLUTE_PATH_BASE: RefCell<Option<PathBuf>> = const { RefCell::new(None) };
|
||||
static ABSOLUTE_PATH_HOME: RefCell<Option<PathBuf>> = const { RefCell::new(None) };
|
||||
}
|
||||
|
||||
/// Ensure this guard is held while deserializing `AbsolutePathBuf` values to
|
||||
@@ -341,6 +344,15 @@ impl AbsolutePathBufGuard {
|
||||
});
|
||||
Self
|
||||
}
|
||||
|
||||
/// Resolves home-relative paths against `home_directory` during `operation`.
|
||||
/// The operation must complete synchronously on the current thread.
|
||||
pub fn with_home_directory<T>(home_directory: &Path, operation: impl FnOnce() -> T) -> T {
|
||||
let previous_home =
|
||||
ABSOLUTE_PATH_HOME.with(|cell| cell.replace(Some(home_directory.to_path_buf())));
|
||||
let _guard = HomeDirectoryGuard(previous_home);
|
||||
operation()
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for AbsolutePathBufGuard {
|
||||
@@ -351,6 +363,16 @@ impl Drop for AbsolutePathBufGuard {
|
||||
}
|
||||
}
|
||||
|
||||
struct HomeDirectoryGuard(Option<PathBuf>);
|
||||
|
||||
impl Drop for HomeDirectoryGuard {
|
||||
fn drop(&mut self) {
|
||||
ABSOLUTE_PATH_HOME.with(|cell| {
|
||||
*cell.borrow_mut() = self.0.take();
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for AbsolutePathBuf {
|
||||
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
|
||||
where
|
||||
@@ -595,6 +617,50 @@ mod tests {
|
||||
assert_eq!(abs_path_buf.as_path(), home.join("code").as_path());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_home_directory_is_used_with_existing_path_guards() {
|
||||
let home_dir = tempdir().expect("explicit home directory");
|
||||
let base_dir = tempdir().expect("base directory");
|
||||
|
||||
let (home_path, relative_path) =
|
||||
AbsolutePathBufGuard::with_home_directory(home_dir.path(), || {
|
||||
let _guard = AbsolutePathBufGuard::new(base_dir.path());
|
||||
let home_path = serde_json::from_str::<AbsolutePathBuf>("\"~/code\"")
|
||||
.expect("deserialize home-relative path");
|
||||
let relative_path = serde_json::from_str::<AbsolutePathBuf>("\"project/file\"")
|
||||
.expect("deserialize relative path");
|
||||
(home_path, relative_path)
|
||||
});
|
||||
|
||||
assert_eq!(home_path.as_path(), home_dir.path().join("code"));
|
||||
assert_eq!(
|
||||
relative_path.as_path(),
|
||||
base_dir.path().join("project/file")
|
||||
);
|
||||
assert!(serde_json::from_str::<AbsolutePathBuf>("\"project/file\"").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nested_explicit_home_directories_restore_the_previous_home() {
|
||||
let outer_home = tempdir().expect("outer home directory");
|
||||
let inner_home = tempdir().expect("inner home directory");
|
||||
|
||||
let (inner_path, restored_path) =
|
||||
AbsolutePathBufGuard::with_home_directory(outer_home.path(), || {
|
||||
let inner_path =
|
||||
AbsolutePathBufGuard::with_home_directory(inner_home.path(), || {
|
||||
AbsolutePathBuf::from_absolute_path("~/project")
|
||||
.expect("resolve path with inner home")
|
||||
});
|
||||
let restored_path = AbsolutePathBuf::from_absolute_path("~/project")
|
||||
.expect("resolve path with restored home");
|
||||
(inner_path, restored_path)
|
||||
});
|
||||
|
||||
assert_eq!(inner_path.as_path(), inner_home.path().join("project"));
|
||||
assert_eq!(restored_path.as_path(), outer_home.path().join("project"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn home_directory_double_slash_is_expanded_in_deserialization() {
|
||||
let Some(home) = home_dir() else {
|
||||
|
||||
Reference in New Issue
Block a user