fix: remove CODEX_MANAGED_CONFIG_PATH environment variable

This commit is contained in:
Michael Bolin
2026-01-05 14:55:39 -08:00
parent 58a91a0b50
commit a2689283bb
9 changed files with 122 additions and 16 deletions

View File

@@ -9,6 +9,7 @@ use codex_app_server_protocol::ConfigWriteResponse;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_core::config::ConfigService;
use codex_core::config::ConfigServiceError;
use codex_core::config_loader::LoaderOverrides;
use serde_json::json;
use std::path::PathBuf;
use toml::Value as TomlValue;
@@ -19,9 +20,13 @@ pub(crate) struct ConfigApi {
}
impl ConfigApi {
pub(crate) fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self {
pub(crate) fn new(
codex_home: PathBuf,
cli_overrides: Vec<(String, TomlValue)>,
loader_overrides: LoaderOverrides,
) -> Self {
Self {
service: ConfigService::new(codex_home, cli_overrides),
service: ConfigService::with_overrides(codex_home, cli_overrides, loader_overrides),
}
}

View File

@@ -1,7 +1,8 @@
#![deny(clippy::print_stdout, clippy::print_stderr)]
use codex_common::CliConfigOverrides;
use codex_core::config::Config;
use codex_core::config::ConfigBuilder;
use codex_core::config_loader::LoaderOverrides;
use std::io::ErrorKind;
use std::io::Result as IoResult;
use std::path::PathBuf;
@@ -42,6 +43,7 @@ const CHANNEL_CAPACITY: usize = 128;
pub async fn run_main(
codex_linux_sandbox_exe: Option<PathBuf>,
cli_config_overrides: CliConfigOverrides,
loader_overrides: LoaderOverrides,
) -> IoResult<()> {
// Set up channels.
let (incoming_tx, mut incoming_rx) = mpsc::channel::<JSONRPCMessage>(CHANNEL_CAPACITY);
@@ -78,7 +80,11 @@ pub async fn run_main(
format!("error parsing -c overrides: {e}"),
)
})?;
let config = Config::load_with_cli_overrides(cli_kv_overrides.clone())
let loader_overrides_for_config_api = loader_overrides.clone();
let config = ConfigBuilder::default()
.cli_overrides(cli_kv_overrides.clone())
.loader_overrides(loader_overrides)
.build()
.await
.map_err(|e| {
std::io::Error::new(ErrorKind::InvalidData, format!("error loading config: {e}"))
@@ -120,11 +126,13 @@ pub async fn run_main(
let processor_handle = tokio::spawn({
let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx);
let cli_overrides: Vec<(String, TomlValue)> = cli_kv_overrides.clone();
let loader_overrides = loader_overrides_for_config_api;
let mut processor = MessageProcessor::new(
outgoing_message_sender,
codex_linux_sandbox_exe,
std::sync::Arc::new(config),
cli_overrides,
loader_overrides,
feedback.clone(),
);
async move {

View File

@@ -1,10 +1,84 @@
use codex_app_server::run_main;
use codex_arg0::arg0_dispatch_or_else;
use codex_common::CliConfigOverrides;
use codex_core::config_loader::LoaderOverrides;
use std::ffi::OsString;
use std::path::PathBuf;
const MANAGED_CONFIG_PATH_FLAG: &str = "--managed-config-path";
const MANAGED_CONFIG_PATH_FLAG_WITH_EQ: &str = "--managed-config-path=";
fn main() -> anyhow::Result<()> {
arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move {
run_main(codex_linux_sandbox_exe, CliConfigOverrides::default()).await?;
// This is intended to be exclusively used via tests: integration tests
// need to point the server at a temporary managed config file without
// writing to /etc.
//
// We intentionally do NOT allow this in release builds because the
// managed config layer is meant to be enterprise-controlled.
let managed_config_path =
managed_config_path_from_args(std::env::args_os(), cfg!(debug_assertions))?;
let loader_overrides = LoaderOverrides {
managed_config_path,
..Default::default()
};
run_main(
codex_linux_sandbox_exe,
CliConfigOverrides::default(),
loader_overrides,
)
.await?;
Ok(())
})
}
fn managed_config_path_from_args(
args: impl IntoIterator<Item = OsString>,
allow_override: bool,
) -> anyhow::Result<Option<PathBuf>> {
let mut args = args.into_iter();
// Skip argv[0].
let _ = args.next();
let mut managed_config_path = None;
while let Some(arg) = args.next() {
if arg == MANAGED_CONFIG_PATH_FLAG {
if !allow_override {
anyhow::bail!("{MANAGED_CONFIG_PATH_FLAG} is not supported in release builds");
}
let value = args.next().ok_or_else(|| {
anyhow::format_err!("{MANAGED_CONFIG_PATH_FLAG} requires a path value")
})?;
managed_config_path = Some(PathBuf::from(value));
continue;
}
if let Some(value) = arg
.to_str()
.and_then(|s| s.strip_prefix(MANAGED_CONFIG_PATH_FLAG_WITH_EQ))
{
if !allow_override {
anyhow::bail!("{MANAGED_CONFIG_PATH_FLAG} is not supported in release builds");
}
managed_config_path = Some(PathBuf::from(value));
}
}
Ok(managed_config_path)
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn flag_constants_are_in_sync() {
// Unfortunately, we cannot derive one from the other using concat!,
// so we hardcode both and test for consistency.
assert_eq!(
format!("{MANAGED_CONFIG_PATH_FLAG}="),
MANAGED_CONFIG_PATH_FLAG_WITH_EQ
);
}
}

View File

@@ -20,6 +20,7 @@ use codex_app_server_protocol::RequestId;
use codex_core::AuthManager;
use codex_core::ConversationManager;
use codex_core::config::Config;
use codex_core::config_loader::LoaderOverrides;
use codex_core::default_client::USER_AGENT_SUFFIX;
use codex_core::default_client::get_codex_user_agent;
use codex_feedback::CodexFeedback;
@@ -41,6 +42,7 @@ impl MessageProcessor {
codex_linux_sandbox_exe: Option<PathBuf>,
config: Arc<Config>,
cli_overrides: Vec<(String, TomlValue)>,
loader_overrides: LoaderOverrides,
feedback: CodexFeedback,
) -> Self {
let outgoing = Arc::new(outgoing);
@@ -62,7 +64,7 @@ impl MessageProcessor {
cli_overrides.clone(),
feedback,
);
let config_api = ConfigApi::new(config.codex_home.clone(), cli_overrides);
let config_api = ConfigApi::new(config.codex_home.clone(), cli_overrides, loader_overrides);
Self {
outgoing,

View File

@@ -67,6 +67,10 @@ impl McpProcess {
Self::new_with_env(codex_home, &[]).await
}
pub async fn new_with_args(codex_home: &Path, args: &[&str]) -> anyhow::Result<Self> {
Self::new_with_args_and_env(codex_home, args, &[]).await
}
/// Creates a new MCP process, allowing tests to override or remove
/// specific environment variables for the child process only.
///
@@ -75,6 +79,16 @@ impl McpProcess {
pub async fn new_with_env(
codex_home: &Path,
env_overrides: &[(&str, Option<&str>)],
) -> anyhow::Result<Self> {
Self::new_with_args_and_env(codex_home, &[], env_overrides).await
}
/// Creates a new MCP process, allowing tests to pass args and override or
/// remove environment variables for the child process only.
pub async fn new_with_args_and_env(
codex_home: &Path,
args: &[&str],
env_overrides: &[(&str, Option<&str>)],
) -> anyhow::Result<Self> {
let program = codex_utils_cargo_bin::cargo_bin("codex-app-server")
.context("should find binary for codex-app-server")?;
@@ -85,6 +99,7 @@ impl McpProcess {
cmd.stderr(Stdio::piped());
cmd.env("CODEX_HOME", codex_home);
cmd.env("RUST_LOG", "debug");
cmd.args(args);
for (k, v) in env_overrides {
match v {

View File

@@ -182,9 +182,9 @@ writable_roots = [{}]
let managed_path_str = managed_path.display().to_string();
let mut mcp = McpProcess::new_with_env(
let mut mcp = McpProcess::new_with_args(
codex_home.path(),
&[("CODEX_MANAGED_CONFIG_PATH", Some(&managed_path_str))],
&["--managed-config-path", &managed_path_str],
)
.await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;

View File

@@ -480,7 +480,12 @@ async fn cli_main(codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()
}
Some(Subcommand::AppServer(app_server_cli)) => match app_server_cli.subcommand {
None => {
codex_app_server::run_main(codex_linux_sandbox_exe, root_config_overrides).await?;
codex_app_server::run_main(
codex_linux_sandbox_exe,
root_config_overrides,
codex_core::config_loader::LoaderOverrides::default(),
)
.await?;
}
Some(AppServerSubcommand::GenerateTs(gen_cli)) => {
codex_app_server_protocol::generate_ts(

View File

@@ -114,8 +114,9 @@ impl ConfigService {
}
}
#[cfg(test)]
fn with_overrides(
/// Construct a service with custom loader overrides (primarily for tests
/// and embedding callers that need to control managed config sources).
pub fn with_overrides(
codex_home: PathBuf,
cli_overrides: Vec<(String, TomlValue)>,
loader_overrides: LoaderOverrides,

View File

@@ -93,12 +93,8 @@ pub(super) async fn read_config_from_path(
}
}
/// Return the default managed config path (honoring `CODEX_MANAGED_CONFIG_PATH`).
/// Return the default managed config path.
pub(super) fn managed_config_default_path(codex_home: &Path) -> PathBuf {
if let Ok(path) = std::env::var("CODEX_MANAGED_CONFIG_PATH") {
return PathBuf::from(path);
}
#[cfg(unix)]
{
let _ = codex_home;