Skip to content

Commit e27d9bd

Browse files
authored
feat: honor /etc/codex/config.toml (#8461)
This adds logic to load `/etc/codex/config.toml` and associate it with `ConfigLayerSource::System` on UNIX. I refactored the code so it shares logic with the creation of the `ConfigLayerSource::User` layer.
1 parent 414fbe0 commit e27d9bd

File tree

6 files changed

+179
-69
lines changed

6 files changed

+179
-69
lines changed

codex-rs/app-server-protocol/src/protocol/v2.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ pub enum ConfigLayerSource {
227227
#[serde(rename_all = "camelCase")]
228228
#[ts(rename_all = "camelCase")]
229229
System {
230+
/// This is the path to the system config.toml file, though it is not
231+
/// guaranteed to exist.
230232
file: AbsolutePathBuf,
231233
},
232234

codex-rs/app-server/tests/suite/v2/config_rpc.rs

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use codex_app_server_protocol::RequestId;
1818
use codex_app_server_protocol::SandboxMode;
1919
use codex_app_server_protocol::ToolsV2;
2020
use codex_app_server_protocol::WriteStatus;
21+
use codex_core::config_loader::SYSTEM_CONFIG_TOML_FILE_UNIX;
2122
use codex_utils_absolute_path::AbsolutePathBuf;
2223
use pretty_assertions::assert_eq;
2324
use serde_json::json;
@@ -73,8 +74,7 @@ sandbox_mode = "workspace-write"
7374
}
7475
);
7576
let layers = layers.expect("layers present");
76-
assert_eq!(layers.len(), 1);
77-
assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file });
77+
assert_layers_user_then_optional_system(&layers, user_file)?;
7878

7979
Ok(())
8080
}
@@ -136,8 +136,7 @@ view_image = false
136136
);
137137

138138
let layers = layers.expect("layers present");
139-
assert_eq!(layers.len(), 1);
140-
assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file });
139+
assert_layers_user_then_optional_system(&layers, user_file)?;
141140

142141
Ok(())
143142
}
@@ -257,12 +256,7 @@ writable_roots = [{}]
257256
);
258257

259258
let layers = layers.expect("layers present");
260-
assert_eq!(layers.len(), 2);
261-
assert_eq!(
262-
layers[0].name,
263-
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
264-
);
265-
assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file });
259+
assert_layers_managed_user_then_optional_system(&layers, managed_file, user_file)?;
266260

267261
Ok(())
268262
}
@@ -433,3 +427,50 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> {
433427

434428
Ok(())
435429
}
430+
431+
fn assert_layers_user_then_optional_system(
432+
layers: &[codex_app_server_protocol::ConfigLayer],
433+
user_file: AbsolutePathBuf,
434+
) -> Result<()> {
435+
if cfg!(unix) {
436+
let system_file = AbsolutePathBuf::from_absolute_path(SYSTEM_CONFIG_TOML_FILE_UNIX)?;
437+
assert_eq!(layers.len(), 2);
438+
assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file });
439+
assert_eq!(
440+
layers[1].name,
441+
ConfigLayerSource::System { file: system_file }
442+
);
443+
} else {
444+
assert_eq!(layers.len(), 1);
445+
assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file });
446+
}
447+
Ok(())
448+
}
449+
450+
fn assert_layers_managed_user_then_optional_system(
451+
layers: &[codex_app_server_protocol::ConfigLayer],
452+
managed_file: AbsolutePathBuf,
453+
user_file: AbsolutePathBuf,
454+
) -> Result<()> {
455+
if cfg!(unix) {
456+
let system_file = AbsolutePathBuf::from_absolute_path(SYSTEM_CONFIG_TOML_FILE_UNIX)?;
457+
assert_eq!(layers.len(), 3);
458+
assert_eq!(
459+
layers[0].name,
460+
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
461+
);
462+
assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file });
463+
assert_eq!(
464+
layers[2].name,
465+
ConfigLayerSource::System { file: system_file }
466+
);
467+
} else {
468+
assert_eq!(layers.len(), 2);
469+
assert_eq!(
470+
layers[0].name,
471+
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
472+
);
473+
assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file });
474+
}
475+
Ok(())
476+
}

codex-rs/core/src/config/service.rs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -778,15 +778,41 @@ remote_compaction = true
778778
},
779779
);
780780
let layers = response.layers.expect("layers present");
781-
assert_eq!(layers.len(), 2, "expected two layers");
782-
assert_eq!(
783-
layers.first().unwrap().name,
784-
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
785-
);
786-
assert_eq!(
787-
layers.get(1).unwrap().name,
788-
ConfigLayerSource::User { file: user_file }
789-
);
781+
if cfg!(unix) {
782+
let system_file = AbsolutePathBuf::from_absolute_path(
783+
crate::config_loader::SYSTEM_CONFIG_TOML_FILE_UNIX,
784+
)
785+
.expect("system file");
786+
assert_eq!(layers.len(), 3, "expected three layers on unix");
787+
assert_eq!(
788+
layers.first().unwrap().name,
789+
ConfigLayerSource::LegacyManagedConfigTomlFromFile {
790+
file: managed_file.clone()
791+
}
792+
);
793+
assert_eq!(
794+
layers.get(1).unwrap().name,
795+
ConfigLayerSource::User {
796+
file: user_file.clone()
797+
}
798+
);
799+
assert_eq!(
800+
layers.get(2).unwrap().name,
801+
ConfigLayerSource::System { file: system_file }
802+
);
803+
} else {
804+
assert_eq!(layers.len(), 2, "expected two layers");
805+
assert_eq!(
806+
layers.first().unwrap().name,
807+
ConfigLayerSource::LegacyManagedConfigTomlFromFile {
808+
file: managed_file.clone()
809+
}
810+
);
811+
assert_eq!(
812+
layers.get(1).unwrap().name,
813+
ConfigLayerSource::User { file: user_file }
814+
);
815+
}
790816
}
791817

792818
#[tokio::test]

codex-rs/core/src/config_loader/mod.rs

Lines changed: 87 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ pub use state::LoaderOverrides;
3333

3434
/// On Unix systems, load requirements from this file path, if present.
3535
const DEFAULT_REQUIREMENTS_TOML_FILE_UNIX: &str = "/etc/codex/requirements.toml";
36+
37+
/// On Unix systems, load default settings from this file path, if present.
38+
/// Note that /etc/codex/ is treated as a "config folder," so subfolders such
39+
/// as skills/ and rules/ will also be honored.
40+
pub const SYSTEM_CONFIG_TOML_FILE_UNIX: &str = "/etc/codex/config.toml";
41+
3642
const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"];
3743

3844
/// To build up the set of admin-enforced constraints, we build up from multiple
@@ -72,7 +78,7 @@ pub async fn load_config_layers_state(
7278
) -> io::Result<ConfigLayerStack> {
7379
let mut config_requirements_toml = ConfigRequirementsToml::default();
7480

75-
// TODO(mbolin): Support an entry in MDM for config requirements and use it
81+
// TODO(gt): Support an entry in MDM for config requirements and use it
7682
// with `config_requirements_toml.merge_unset_fields(...)`, if present.
7783

7884
// Honor /etc/codex/requirements.toml.
@@ -95,57 +101,45 @@ pub async fn load_config_layers_state(
95101

96102
let mut layers = Vec::<ConfigLayerEntry>::new();
97103

98-
// TODO(mbolin): Honor managed preferences (macOS only).
99-
// TODO(mbolin): Honor /etc/codex/config.toml.
104+
// TODO(gt): Honor managed preferences (macOS only).
105+
106+
// Include an entry for the "system" config folder, loading its config.toml,
107+
// if it exists.
108+
let system_config_toml_file = if cfg!(unix) {
109+
Some(AbsolutePathBuf::from_absolute_path(
110+
SYSTEM_CONFIG_TOML_FILE_UNIX,
111+
)?)
112+
} else {
113+
// TODO(gt): Determine the path to load on Windows.
114+
None
115+
};
116+
if let Some(system_config_toml_file) = system_config_toml_file {
117+
let system_layer =
118+
load_config_toml_for_required_layer(&system_config_toml_file, |config_toml| {
119+
ConfigLayerEntry::new(
120+
ConfigLayerSource::System {
121+
file: system_config_toml_file.clone(),
122+
},
123+
config_toml,
124+
)
125+
})
126+
.await?;
127+
layers.push(system_layer);
128+
}
100129

101130
// Add a layer for $CODEX_HOME/config.toml if it exists. Note if the file
102131
// exists, but is malformed, then this error should be propagated to the
103132
// user.
104133
let user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home)?;
105-
let user_layer = match tokio::fs::read_to_string(&user_file).await {
106-
Ok(contents) => {
107-
let user_config: TomlValue = toml::from_str(&contents).map_err(|e| {
108-
io::Error::new(
109-
io::ErrorKind::InvalidData,
110-
format!(
111-
"Error parsing user config file {}: {e}",
112-
user_file.as_path().display(),
113-
),
114-
)
115-
})?;
116-
let user_config_parent = user_file.as_path().parent().ok_or_else(|| {
117-
io::Error::new(
118-
io::ErrorKind::InvalidData,
119-
format!(
120-
"User config file {} has no parent directory",
121-
user_file.as_path().display()
122-
),
123-
)
124-
})?;
125-
let user_config =
126-
resolve_relative_paths_in_config_toml(user_config, user_config_parent)?;
127-
ConfigLayerEntry::new(ConfigLayerSource::User { file: user_file }, user_config)
128-
}
129-
Err(e) => {
130-
if e.kind() == io::ErrorKind::NotFound {
131-
// If there is no config.toml file, record an empty entry
132-
// for this user layer, as this may still have subfolders
133-
// that are significant in the overall ConfigLayerStack.
134-
ConfigLayerEntry::new(
135-
ConfigLayerSource::User { file: user_file },
136-
TomlValue::Table(toml::map::Map::new()),
137-
)
138-
} else {
139-
return Err(io::Error::new(
140-
e.kind(),
141-
format!(
142-
"Failed to read user config file {}: {e}",
143-
user_file.as_path().display(),
144-
),
145-
));
146-
}
147-
}
148-
};
134+
let user_layer = load_config_toml_for_required_layer(&user_file, |config_toml| {
135+
ConfigLayerEntry::new(
136+
ConfigLayerSource::User {
137+
file: user_file.clone(),
138+
},
139+
config_toml,
140+
)
141+
})
142+
.await?;
149143
layers.push(user_layer);
150144

151145
if let Some(cwd) = cwd {
@@ -206,6 +200,52 @@ pub async fn load_config_layers_state(
206200
ConfigLayerStack::new(layers, config_requirements_toml.try_into()?)
207201
}
208202

203+
/// Attempts to load a config.toml file from `config_toml`.
204+
/// - If the file exists and is valid TOML, passes the parsed `toml::Value` to
205+
/// `create_entry` and returns the resulting layer entry.
206+
/// - If the file does not exist, uses an empty `Table` with `create_entry` and
207+
/// returns the resulting layer entry.
208+
/// - If there is an error reading the file or parsing the TOML, returns an
209+
/// error.
210+
async fn load_config_toml_for_required_layer(
211+
config_toml: impl AsRef<Path>,
212+
create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry,
213+
) -> io::Result<ConfigLayerEntry> {
214+
let toml_file = config_toml.as_ref();
215+
let toml_value = match tokio::fs::read_to_string(toml_file).await {
216+
Ok(contents) => {
217+
let config: TomlValue = toml::from_str(&contents).map_err(|e| {
218+
io::Error::new(
219+
io::ErrorKind::InvalidData,
220+
format!("Error parsing config file {}: {e}", toml_file.display()),
221+
)
222+
})?;
223+
let config_parent = toml_file.parent().ok_or_else(|| {
224+
io::Error::new(
225+
io::ErrorKind::InvalidData,
226+
format!(
227+
"Config file {} has no parent directory",
228+
toml_file.display()
229+
),
230+
)
231+
})?;
232+
resolve_relative_paths_in_config_toml(config, config_parent)
233+
}
234+
Err(e) => {
235+
if e.kind() == io::ErrorKind::NotFound {
236+
Ok(TomlValue::Table(toml::map::Map::new()))
237+
} else {
238+
Err(io::Error::new(
239+
e.kind(),
240+
format!("Failed to read config file {}: {e}", toml_file.display()),
241+
))
242+
}
243+
}
244+
}?;
245+
246+
Ok(create_entry(toml_value))
247+
}
248+
209249
/// If available, apply requirements from `/etc/codex/requirements.toml` to
210250
/// `config_requirements_toml` by filling in any unset fields.
211251
async fn load_requirements_toml(

codex-rs/core/src/config_loader/state.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl ConfigLayerEntry {
5555
pub fn config_folder(&self) -> Option<AbsolutePathBuf> {
5656
match &self.name {
5757
ConfigLayerSource::Mdm { .. } => None,
58-
ConfigLayerSource::System { .. } => None,
58+
ConfigLayerSource::System { file } => file.parent(),
5959
ConfigLayerSource::User { file } => file.parent(),
6060
ConfigLayerSource::Project { dot_codex_folder } => Some(dot_codex_folder.clone()),
6161
ConfigLayerSource::SessionFlags => None,

codex-rs/core/src/config_loader/tests.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,10 @@ async fn returns_empty_when_all_layers_missing() {
119119
.iter()
120120
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::System { .. }))
121121
.count();
122+
let expected_system_layers = if cfg!(unix) { 1 } else { 0 };
122123
assert_eq!(
123-
num_system_layers, 0,
124-
"managed config layer should be absent when file missing"
124+
num_system_layers, expected_system_layers,
125+
"system layer should be present only on unix"
125126
);
126127

127128
#[cfg(not(target_os = "macos"))]

0 commit comments

Comments
 (0)