mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
inline hostname resolution for remote sandbox config (#19739)
# Why Requirements support host-specific `remote_sandbox_config.hostname_patterns`, but config loading previously resolved and passed the system hostname through every config-loading path even when no requirements layer used `remote_sandbox_config`. On machines where hostname lookup is slow, startup and app-server config reads paid for a feature that was not active. We only need the hostname when a requirements layer actually declares `remote_sandbox_config`, so this moves hostname resolution to the single requirements merge point and keeps all other config callers unaware of hostname matching. # What - Removed the eager `host_name` plumbing from `load_config_layers_state`, `load_requirements_toml`, `ConfigBuilder`, app-server `ConfigManager`, network proxy loading, and related call sites. - Resolve the hostname inside `merge_requirements_with_remote_sandbox_config` only when the incoming requirements contain `remote_sandbox_config`.
This commit is contained in:
@@ -28,6 +28,7 @@ use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -107,7 +108,6 @@ async fn returns_config_error_for_invalid_user_config_toml() {
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("expected error");
|
||||
@@ -139,7 +139,6 @@ async fn ignore_user_config_keeps_empty_user_layer() -> std::io::Result<()> {
|
||||
},
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -171,7 +170,6 @@ async fn ignore_rules_marks_config_stack_for_exec_policy_rule_skip() -> std::io:
|
||||
},
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -197,7 +195,6 @@ async fn returns_config_error_for_invalid_managed_config_toml() {
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("expected error");
|
||||
@@ -284,7 +281,6 @@ extra = true
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("load config");
|
||||
@@ -319,7 +315,6 @@ async fn returns_empty_when_all_layers_missing() {
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("load layers");
|
||||
@@ -395,7 +390,6 @@ async fn includes_thread_config_layers_in_stack() -> anyhow::Result<()> {
|
||||
features: BTreeMap::from([("plugins".to_string(), false)]),
|
||||
..Default::default()
|
||||
})]),
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -472,7 +466,6 @@ flag = false
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("load config");
|
||||
@@ -576,7 +569,6 @@ allowed_sandbox_modes = ["read-only"]
|
||||
loader_overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -639,7 +631,6 @@ allowed_approval_policies = ["never"]
|
||||
loader_overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -681,7 +672,6 @@ personality = true
|
||||
LOCAL_FS.as_ref(),
|
||||
&mut config_requirements_toml,
|
||||
&requirements_file,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -797,7 +787,6 @@ allowed_approval_policies = ["on-request"]
|
||||
}))
|
||||
}),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -857,7 +846,6 @@ allowed_approval_policies = ["on-request"]
|
||||
LOCAL_FS.as_ref(),
|
||||
&mut config_requirements_toml,
|
||||
&AbsolutePathBuf::try_from(requirements_file)?,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -879,54 +867,6 @@ allowed_approval_policies = ["on-request"]
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn system_remote_sandbox_config_keeps_cloud_sandbox_modes() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
let requirements_file = tmp.path().join("requirements.toml");
|
||||
tokio::fs::write(
|
||||
&requirements_file,
|
||||
r#"
|
||||
[[remote_sandbox_config]]
|
||||
hostname_patterns = ["runner-*.ci.example.com"]
|
||||
allowed_sandbox_modes = ["read-only", "workspace-write"]
|
||||
"#,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let cloud_source = RequirementSource::CloudRequirements;
|
||||
let mut config_requirements_toml = ConfigRequirementsWithSources::default();
|
||||
config_requirements_toml.merge_unset_fields(
|
||||
cloud_source.clone(),
|
||||
toml::from_str(
|
||||
r#"
|
||||
allowed_sandbox_modes = ["read-only"]
|
||||
"#,
|
||||
)?,
|
||||
);
|
||||
load_requirements_toml(
|
||||
LOCAL_FS.as_ref(),
|
||||
&mut config_requirements_toml,
|
||||
&AbsolutePathBuf::try_from(requirements_file)?,
|
||||
Some("runner-01.ci.example.com"),
|
||||
)
|
||||
.await?;
|
||||
let config_requirements: ConfigRequirements = config_requirements_toml.try_into()?;
|
||||
|
||||
assert_eq!(
|
||||
config_requirements
|
||||
.sandbox_policy
|
||||
.can_set(&SandboxPolicy::new_workspace_write_policy()),
|
||||
Err(ConstraintError::InvalidValue {
|
||||
field_name: "sandbox_mode",
|
||||
candidate: "WorkspaceWrite".into(),
|
||||
allowed: "[ReadOnly]".into(),
|
||||
requirement_source: cloud_source,
|
||||
})
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn load_requirements_toml_resolves_deny_read_against_parent() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
@@ -948,7 +888,6 @@ deny_read = ["./sensitive", "../shared/secret.txt"]
|
||||
LOCAL_FS.as_ref(),
|
||||
&mut config_requirements_toml,
|
||||
&requirements_file,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1003,7 +942,6 @@ deny_read = ["./sensitive/**/*.txt"]
|
||||
LOCAL_FS.as_ref(),
|
||||
&mut config_requirements_toml,
|
||||
&requirements_file,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1072,7 +1010,6 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()>
|
||||
LoaderOverrides::default(),
|
||||
cloud_requirements,
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1135,7 +1072,6 @@ async fn load_config_layers_includes_cloud_hook_requirements() -> anyhow::Result
|
||||
LoaderOverrides::default(),
|
||||
cloud_requirements,
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1152,53 +1088,6 @@ async fn load_config_layers_includes_cloud_hook_requirements() -> anyhow::Result
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_layers_applies_matching_remote_sandbox_config() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
let codex_home = tmp.path().join("home");
|
||||
tokio::fs::create_dir_all(&codex_home).await?;
|
||||
let cwd = AbsolutePathBuf::from_absolute_path(tmp.path())?;
|
||||
|
||||
let requirements: ConfigRequirementsToml = toml::from_str(
|
||||
r#"
|
||||
allowed_sandbox_modes = ["read-only"]
|
||||
|
||||
[[remote_sandbox_config]]
|
||||
hostname_patterns = ["runner-*.ci.example.com"]
|
||||
allowed_sandbox_modes = ["read-only", "workspace-write"]
|
||||
"#,
|
||||
)?;
|
||||
let cloud_requirements = CloudRequirementsLoader::new(async move { Ok(Some(requirements)) });
|
||||
let layers = load_config_layers_state(
|
||||
LOCAL_FS.as_ref(),
|
||||
&codex_home,
|
||||
Some(cwd),
|
||||
&[] as &[(String, TomlValue)],
|
||||
LoaderOverrides::default(),
|
||||
cloud_requirements,
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
Some("runner-01.ci.example.com"),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
layers.requirements_toml().allowed_sandbox_modes,
|
||||
Some(vec![
|
||||
codex_config::SandboxModeRequirement::ReadOnly,
|
||||
codex_config::SandboxModeRequirement::WorkspaceWrite,
|
||||
])
|
||||
);
|
||||
assert!(
|
||||
layers
|
||||
.requirements()
|
||||
.sandbox_policy
|
||||
.can_set(&SandboxPolicy::new_workspace_write_policy())
|
||||
.is_ok()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_layers_fails_when_cloud_requirements_loader_fails() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
@@ -1220,7 +1109,6 @@ async fn load_config_layers_fails_when_cloud_requirements_loader_fails() -> anyh
|
||||
))
|
||||
}),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("cloud requirements failure should fail closed");
|
||||
@@ -1269,7 +1157,6 @@ async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> {
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1416,7 +1303,6 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1458,7 +1344,6 @@ async fn codex_home_is_not_loaded_as_project_layer_from_home_dir() -> std::io::R
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1517,7 +1402,6 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1590,7 +1474,6 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result<
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
let project_layers_untrusted: Vec<_> = layers_untrusted
|
||||
@@ -1631,7 +1514,6 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result<
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
let project_layers_unknown: Vec<_> = layers_unknown
|
||||
@@ -1699,7 +1581,6 @@ async fn project_trust_does_not_match_configured_alias_for_canonical_cwd() -> st
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1854,7 +1735,6 @@ async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io::
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
let project_layers: Vec<_> = layers
|
||||
@@ -1924,7 +1804,6 @@ async fn project_layer_without_config_toml_is_disabled_when_untrusted_or_unknown
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
let project_layers: Vec<_> = layers
|
||||
@@ -1986,7 +1865,6 @@ async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> std::io
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -2031,7 +1909,6 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<()
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -2538,7 +2538,6 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> {
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
let cfg =
|
||||
@@ -2674,7 +2673,6 @@ async fn managed_config_wins_over_cli_overrides() -> anyhow::Result<()> {
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -759,7 +759,7 @@ impl AuthManagerConfig for Config {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
#[derive(Clone, Default)]
|
||||
pub struct ConfigBuilder {
|
||||
codex_home: Option<PathBuf>,
|
||||
cli_overrides: Option<Vec<(String, TomlValue)>>,
|
||||
@@ -768,22 +768,6 @@ pub struct ConfigBuilder {
|
||||
cloud_requirements: CloudRequirementsLoader,
|
||||
thread_config_loader: Option<Arc<dyn ThreadConfigLoader>>,
|
||||
fallback_cwd: Option<PathBuf>,
|
||||
host_name: Option<String>,
|
||||
}
|
||||
|
||||
impl Default for ConfigBuilder {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
codex_home: None,
|
||||
cli_overrides: None,
|
||||
harness_overrides: None,
|
||||
loader_overrides: None,
|
||||
cloud_requirements: CloudRequirementsLoader::default(),
|
||||
thread_config_loader: None,
|
||||
fallback_cwd: None,
|
||||
host_name: codex_config::host_name(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ConfigBuilder {
|
||||
@@ -825,11 +809,6 @@ impl ConfigBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
pub fn host_name(mut self, host_name: Option<String>) -> Self {
|
||||
self.host_name = host_name;
|
||||
self
|
||||
}
|
||||
|
||||
pub async fn build(self) -> std::io::Result<Config> {
|
||||
let Self {
|
||||
codex_home,
|
||||
@@ -839,7 +818,6 @@ impl ConfigBuilder {
|
||||
cloud_requirements,
|
||||
thread_config_loader,
|
||||
fallback_cwd,
|
||||
host_name,
|
||||
} = self;
|
||||
let codex_home = match codex_home {
|
||||
Some(codex_home) => AbsolutePathBuf::from_absolute_path(codex_home)?,
|
||||
@@ -864,7 +842,6 @@ impl ConfigBuilder {
|
||||
thread_config_loader
|
||||
.as_deref()
|
||||
.unwrap_or(&codex_config::NoopThreadConfigLoader),
|
||||
host_name.as_deref(),
|
||||
)
|
||||
.await?;
|
||||
let merged_toml = config_layer_stack.effective_config();
|
||||
@@ -1047,7 +1024,6 @@ pub async fn load_config_as_toml_with_cli_and_loader_overrides(
|
||||
loader_overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1229,7 +1205,6 @@ pub async fn load_global_mcp_servers(
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await?;
|
||||
let merged_toml = config_layer_stack.effective_config();
|
||||
|
||||
@@ -54,7 +54,6 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec<LayerMtime
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
.context("failed to load Codex config")?;
|
||||
|
||||
@@ -601,7 +601,6 @@ pub async fn list_skills(sess: &Session, sub_id: String, cwds: Vec<PathBuf>, for
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
/*host_name*/ None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user