fix: stop honoring CODEX_MANAGED_CONFIG_PATH environment variable in production (#8762)
This commit is contained in:
parent
8858012fd1
commit
7ecd0dc9b3
9 changed files with 83 additions and 33 deletions
|
|
@ -1103,7 +1103,7 @@ impl CodexMessageProcessor {
|
|||
}
|
||||
|
||||
async fn get_user_saved_config(&self, request_id: RequestId) {
|
||||
let service = ConfigService::new(self.config.codex_home.clone(), Vec::new());
|
||||
let service = ConfigService::new_with_defaults(self.config.codex_home.clone());
|
||||
let user_saved_config: UserSavedConfig = match service.load_user_saved_config().await {
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
|
|
|
|||
|
|
@ -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::new(codex_home, cli_overrides, loader_overrides),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -1,10 +1,42 @@
|
|||
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::path::PathBuf;
|
||||
|
||||
// Debug-only test hook: lets integration tests point the server at a temporary
|
||||
// managed config file without writing to /etc.
|
||||
const MANAGED_CONFIG_PATH_ENV_VAR: &str = "CODEX_APP_SERVER_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?;
|
||||
let managed_config_path = managed_config_path_from_debug_env();
|
||||
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_debug_env() -> Option<PathBuf> {
|
||||
#[cfg(debug_assertions)]
|
||||
{
|
||||
if let Ok(value) = std::env::var(MANAGED_CONFIG_PATH_ENV_VAR) {
|
||||
return if value.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(PathBuf::from(value))
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -184,7 +184,10 @@ writable_roots = [{}]
|
|||
|
||||
let mut mcp = McpProcess::new_with_env(
|
||||
codex_home.path(),
|
||||
&[("CODEX_MANAGED_CONFIG_PATH", Some(&managed_path_str))],
|
||||
&[(
|
||||
"CODEX_APP_SERVER_MANAGED_CONFIG_PATH",
|
||||
Some(&managed_path_str),
|
||||
)],
|
||||
)
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -106,16 +106,7 @@ pub struct ConfigService {
|
|||
}
|
||||
|
||||
impl ConfigService {
|
||||
pub fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self {
|
||||
Self {
|
||||
codex_home,
|
||||
cli_overrides,
|
||||
loader_overrides: LoaderOverrides::default(),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn with_overrides(
|
||||
pub fn new(
|
||||
codex_home: PathBuf,
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
loader_overrides: LoaderOverrides,
|
||||
|
|
@ -127,6 +118,14 @@ impl ConfigService {
|
|||
}
|
||||
}
|
||||
|
||||
pub fn new_with_defaults(codex_home: PathBuf) -> Self {
|
||||
Self {
|
||||
codex_home,
|
||||
cli_overrides: Vec::new(),
|
||||
loader_overrides: LoaderOverrides::default(),
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn read(
|
||||
&self,
|
||||
params: ConfigReadParams,
|
||||
|
|
@ -707,7 +706,7 @@ unified_exec = true
|
|||
"#;
|
||||
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), original)?;
|
||||
|
||||
let service = ConfigService::new(tmp.path().to_path_buf(), vec![]);
|
||||
let service = ConfigService::new_with_defaults(tmp.path().to_path_buf());
|
||||
service
|
||||
.write_value(ConfigValueWriteParams {
|
||||
file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()),
|
||||
|
|
@ -748,7 +747,7 @@ remote_compaction = true
|
|||
std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap();
|
||||
let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file");
|
||||
|
||||
let service = ConfigService::with_overrides(
|
||||
let service = ConfigService::new(
|
||||
tmp.path().to_path_buf(),
|
||||
vec![],
|
||||
LoaderOverrides {
|
||||
|
|
@ -829,7 +828,7 @@ remote_compaction = true
|
|||
std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap();
|
||||
let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file");
|
||||
|
||||
let service = ConfigService::with_overrides(
|
||||
let service = ConfigService::new(
|
||||
tmp.path().to_path_buf(),
|
||||
vec![],
|
||||
LoaderOverrides {
|
||||
|
|
@ -881,7 +880,7 @@ remote_compaction = true
|
|||
let user_path = tmp.path().join(CONFIG_TOML_FILE);
|
||||
std::fs::write(&user_path, "model = \"user\"").unwrap();
|
||||
|
||||
let service = ConfigService::new(tmp.path().to_path_buf(), vec![]);
|
||||
let service = ConfigService::new_with_defaults(tmp.path().to_path_buf());
|
||||
let error = service
|
||||
.write_value(ConfigValueWriteParams {
|
||||
file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()),
|
||||
|
|
@ -904,7 +903,7 @@ remote_compaction = true
|
|||
let tmp = tempdir().expect("tempdir");
|
||||
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap();
|
||||
|
||||
let service = ConfigService::new(tmp.path().to_path_buf(), vec![]);
|
||||
let service = ConfigService::new_with_defaults(tmp.path().to_path_buf());
|
||||
service
|
||||
.write_value(ConfigValueWriteParams {
|
||||
file_path: None,
|
||||
|
|
@ -932,7 +931,7 @@ remote_compaction = true
|
|||
let managed_path = tmp.path().join("managed_config.toml");
|
||||
std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap();
|
||||
|
||||
let service = ConfigService::with_overrides(
|
||||
let service = ConfigService::new(
|
||||
tmp.path().to_path_buf(),
|
||||
vec![],
|
||||
LoaderOverrides {
|
||||
|
|
@ -980,7 +979,7 @@ remote_compaction = true
|
|||
TomlValue::String("session".to_string()),
|
||||
)];
|
||||
|
||||
let service = ConfigService::with_overrides(
|
||||
let service = ConfigService::new(
|
||||
tmp.path().to_path_buf(),
|
||||
cli_overrides,
|
||||
LoaderOverrides {
|
||||
|
|
@ -1026,7 +1025,7 @@ remote_compaction = true
|
|||
std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap();
|
||||
let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file");
|
||||
|
||||
let service = ConfigService::with_overrides(
|
||||
let service = ConfigService::new(
|
||||
tmp.path().to_path_buf(),
|
||||
vec![],
|
||||
LoaderOverrides {
|
||||
|
|
@ -1085,7 +1084,7 @@ alpha = "a"
|
|||
|
||||
std::fs::write(&path, base)?;
|
||||
|
||||
let service = ConfigService::new(tmp.path().to_path_buf(), vec![]);
|
||||
let service = ConfigService::new_with_defaults(tmp.path().to_path_buf());
|
||||
service
|
||||
.write_value(ConfigValueWriteParams {
|
||||
file_path: Some(path.display().to_string()),
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue