Print warning if we skip config loading (#9611)

https://github.com/openai/codex/pull/9533 silently ignored config if
untrusted. Instead, we still load it but disable it. Maybe we shouldn't
try to parse it either...

<img width="939" height="515" alt="Screenshot 2026-01-21 at 14 56 38"
src="https://github.com/user-attachments/assets/e753cc22-dd99-4242-8ffe-7589e85bef66"
/>
This commit is contained in:
gt-oai
2026-01-23 20:06:37 +00:00
committed by GitHub
parent eca365cf8c
commit 7938c170d9
10 changed files with 424 additions and 78 deletions

View File

@@ -16,7 +16,7 @@ Exported from `codex_core::config_loader`:
- `origins() -> HashMap<String, ConfigLayerMetadata>`
- `layers_high_to_low() -> Vec<ConfigLayer>`
- `with_user_config(user_config) -> ConfigLayerStack`
- `ConfigLayerEntry` (one layers `{name, config, version}`; `name` carries source metadata)
- `ConfigLayerEntry` (one layers `{name, config, version, disabled_reason}`; `name` carries source metadata)
- `LoaderOverrides` (test/override hooks for managed config sources)
- `merge_toml_values(base, overlay)` (public helper used elsewhere)
@@ -29,7 +29,9 @@ Precedence is **top overrides bottom**:
3. **Session flags** (CLI overrides, applied as dotted-path TOML writes)
4. **User** config (`config.toml`)
This is what `ConfigLayerStack::effective_config()` implements.
Layers with a `disabled_reason` are still surfaced for UI, but are ignored when
computing the effective config and origins metadata. This is what
`ConfigLayerStack::effective_config()` implements.
## Typical usage

View File

@@ -67,9 +67,9 @@ const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"];
/// - admin: managed preferences (*)
/// - system `/etc/codex/config.toml`
/// - user `${CODEX_HOME}/config.toml`
/// - cwd `${PWD}/config.toml` (only when the directory is trusted)
/// - tree parent directories up to root looking for `./.codex/config.toml` (trusted only)
/// - repo `$(git rev-parse --show-toplevel)/.codex/config.toml` (trusted only)
/// - cwd `${PWD}/config.toml` (loaded but disabled when the directory is untrusted)
/// - tree parent directories up to root looking for `./.codex/config.toml` (loaded but disabled when untrusted)
/// - repo `$(git rev-parse --show-toplevel)/.codex/config.toml` (loaded but disabled when untrusted)
/// - runtime e.g., --config flags, model selector in UI
///
/// (*) Only available on macOS via managed device profiles.
@@ -173,12 +173,21 @@ pub async fn load_config_layers_state(
let project_root_markers = project_root_markers_from_config(&merged_so_far)?
.unwrap_or_else(default_project_root_markers);
if let Some(project_root) =
trusted_project_root(&merged_so_far, &cwd, &project_root_markers, codex_home).await?
{
let project_layers = load_project_layers(&cwd, &project_root).await?;
layers.extend(project_layers);
}
let project_trust_context = project_trust_context(
&merged_so_far,
&cwd,
&project_root_markers,
codex_home,
&user_file,
)
.await?;
let project_layers = load_project_layers(
&cwd,
&project_trust_context.project_root,
&project_trust_context,
)
.await?;
layers.extend(project_layers);
}
// Add a layer for runtime overrides from the CLI or UI, if any exist.
@@ -402,42 +411,132 @@ fn default_project_root_markers() -> Vec<String> {
.collect()
}
async fn trusted_project_root(
struct ProjectTrustContext {
project_root: AbsolutePathBuf,
project_root_key: String,
repo_root_key: Option<String>,
projects_trust: std::collections::HashMap<String, TrustLevel>,
user_config_file: AbsolutePathBuf,
}
struct ProjectTrustDecision {
trust_level: Option<TrustLevel>,
trust_key: String,
}
impl ProjectTrustDecision {
fn is_trusted(&self) -> bool {
matches!(self.trust_level, Some(TrustLevel::Trusted))
}
}
impl ProjectTrustContext {
fn decision_for_dir(&self, dir: &AbsolutePathBuf) -> ProjectTrustDecision {
let dir_key = dir.as_path().to_string_lossy().to_string();
if let Some(trust_level) = self.projects_trust.get(&dir_key).copied() {
return ProjectTrustDecision {
trust_level: Some(trust_level),
trust_key: dir_key,
};
}
if let Some(trust_level) = self.projects_trust.get(&self.project_root_key).copied() {
return ProjectTrustDecision {
trust_level: Some(trust_level),
trust_key: self.project_root_key.clone(),
};
}
if let Some(repo_root_key) = self.repo_root_key.as_ref()
&& let Some(trust_level) = self.projects_trust.get(repo_root_key).copied()
{
return ProjectTrustDecision {
trust_level: Some(trust_level),
trust_key: repo_root_key.clone(),
};
}
ProjectTrustDecision {
trust_level: None,
trust_key: self
.repo_root_key
.clone()
.unwrap_or_else(|| self.project_root_key.clone()),
}
}
fn disabled_reason_for_dir(&self, dir: &AbsolutePathBuf) -> Option<String> {
let decision = self.decision_for_dir(dir);
if decision.is_trusted() {
return None;
}
let trust_key = decision.trust_key.as_str();
let user_config_file = self.user_config_file.as_path().display();
match decision.trust_level {
Some(TrustLevel::Untrusted) => Some(format!(
"{trust_key} is marked as untrusted in {user_config_file}. Mark it trusted to enable project config folders."
)),
_ => Some(format!(
"Add {trust_key} as a trusted project in {user_config_file}."
)),
}
}
}
fn project_layer_entry(
trust_context: &ProjectTrustContext,
dot_codex_folder: &AbsolutePathBuf,
layer_dir: &AbsolutePathBuf,
config: TomlValue,
) -> ConfigLayerEntry {
match trust_context.disabled_reason_for_dir(layer_dir) {
Some(reason) => ConfigLayerEntry::new_disabled(
ConfigLayerSource::Project {
dot_codex_folder: dot_codex_folder.clone(),
},
config,
reason,
),
None => ConfigLayerEntry::new(
ConfigLayerSource::Project {
dot_codex_folder: dot_codex_folder.clone(),
},
config,
),
}
}
async fn project_trust_context(
merged_config: &TomlValue,
cwd: &AbsolutePathBuf,
project_root_markers: &[String],
config_base_dir: &Path,
) -> io::Result<Option<AbsolutePathBuf>> {
user_config_file: &AbsolutePathBuf,
) -> io::Result<ProjectTrustContext> {
let config_toml = deserialize_config_toml_with_base(merged_config.clone(), config_base_dir)?;
let project_root = find_project_root(cwd, project_root_markers).await?;
let projects = config_toml.projects.unwrap_or_default();
let cwd_key = cwd.as_path().to_string_lossy().to_string();
let project_root_key = project_root.as_path().to_string_lossy().to_string();
let repo_root_key = resolve_root_git_project_for_trust(cwd.as_path())
let repo_root = resolve_root_git_project_for_trust(cwd.as_path());
let repo_root_key = repo_root
.as_ref()
.map(|root| root.to_string_lossy().to_string());
let trust_level = projects
.get(&cwd_key)
.and_then(|project| project.trust_level)
.or_else(|| {
projects
.get(&project_root_key)
.and_then(|project| project.trust_level)
})
.or_else(|| {
repo_root_key
.as_ref()
.and_then(|root| projects.get(root))
.and_then(|project| project.trust_level)
});
let projects_trust = projects
.into_iter()
.filter_map(|(key, project)| project.trust_level.map(|trust_level| (key, trust_level)))
.collect();
if matches!(trust_level, Some(TrustLevel::Trusted)) {
Ok(Some(project_root))
} else {
Ok(None)
}
Ok(ProjectTrustContext {
project_root,
project_root_key,
repo_root_key,
projects_trust,
user_config_file: user_config_file.clone(),
})
}
/// Takes a `toml::Value` parsed from a config.toml file and walks through it,
@@ -527,6 +626,7 @@ async fn find_project_root(
async fn load_project_layers(
cwd: &AbsolutePathBuf,
project_root: &AbsolutePathBuf,
trust_context: &ProjectTrustContext,
) -> io::Result<Vec<ConfigLayerEntry>> {
let mut dirs = cwd
.as_path()
@@ -555,46 +655,54 @@ async fn load_project_layers(
continue;
}
let layer_dir = AbsolutePathBuf::from_absolute_path(dir)?;
let decision = trust_context.decision_for_dir(&layer_dir);
let dot_codex_abs = AbsolutePathBuf::from_absolute_path(&dot_codex)?;
let config_file = dot_codex_abs.join(CONFIG_TOML_FILE)?;
match tokio::fs::read_to_string(&config_file).await {
Ok(contents) => {
let config: TomlValue = toml::from_str(&contents).map_err(|e| {
io::Error::new(
io::ErrorKind::InvalidData,
format!(
"Error parsing project config file {}: {e}",
config_file.as_path().display(),
),
)
})?;
let config: TomlValue = match toml::from_str(&contents) {
Ok(config) => config,
Err(e) => {
if decision.is_trusted() {
let config_file_display = config_file.as_path().display();
return Err(io::Error::new(
io::ErrorKind::InvalidData,
format!(
"Error parsing project config file {config_file_display}: {e}"
),
));
}
layers.push(project_layer_entry(
trust_context,
&dot_codex_abs,
&layer_dir,
TomlValue::Table(toml::map::Map::new()),
));
continue;
}
};
let config =
resolve_relative_paths_in_config_toml(config, dot_codex_abs.as_path())?;
layers.push(ConfigLayerEntry::new(
ConfigLayerSource::Project {
dot_codex_folder: dot_codex_abs,
},
config,
));
let entry = project_layer_entry(trust_context, &dot_codex_abs, &layer_dir, config);
layers.push(entry);
}
Err(err) => {
if err.kind() == io::ErrorKind::NotFound {
// If there is no config.toml file, record an empty entry
// for this project layer, as this may still have subfolders
// that are significant in the overall ConfigLayerStack.
layers.push(ConfigLayerEntry::new(
ConfigLayerSource::Project {
dot_codex_folder: dot_codex_abs,
},
layers.push(project_layer_entry(
trust_context,
&dot_codex_abs,
&layer_dir,
TomlValue::Table(toml::map::Map::new()),
));
} else {
let config_file_display = config_file.as_path().display();
return Err(io::Error::new(
err.kind(),
format!(
"Failed to read project config file {}: {err}",
config_file.as_path().display(),
),
format!("Failed to read project config file {config_file_display}: {err}"),
));
}
}

View File

@@ -28,6 +28,7 @@ pub struct ConfigLayerEntry {
pub name: ConfigLayerSource,
pub config: TomlValue,
pub version: String,
pub disabled_reason: Option<String>,
}
impl ConfigLayerEntry {
@@ -37,9 +38,28 @@ impl ConfigLayerEntry {
name,
config,
version,
disabled_reason: None,
}
}
pub fn new_disabled(
name: ConfigLayerSource,
config: TomlValue,
disabled_reason: impl Into<String>,
) -> Self {
let version = version_for_toml(&config);
Self {
name,
config,
version,
disabled_reason: Some(disabled_reason.into()),
}
}
pub fn is_disabled(&self) -> bool {
self.disabled_reason.is_some()
}
pub fn metadata(&self) -> ConfigLayerMetadata {
ConfigLayerMetadata {
name: self.name.clone(),
@@ -52,6 +72,7 @@ impl ConfigLayerEntry {
name: self.name.clone(),
version: self.version.clone(),
config: serde_json::to_value(&self.config).unwrap_or(JsonValue::Null),
disabled_reason: self.disabled_reason.clone(),
}
}
@@ -172,7 +193,7 @@ impl ConfigLayerStack {
pub fn effective_config(&self) -> TomlValue {
let mut merged = TomlValue::Table(toml::map::Map::new());
for layer in &self.layers {
for layer in self.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false) {
merge_toml_values(&mut merged, &layer.config);
}
merged
@@ -182,7 +203,7 @@ impl ConfigLayerStack {
let mut origins = HashMap::new();
let mut path = Vec::new();
for layer in &self.layers {
for layer in self.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false) {
record_origins(&layer.config, &layer.metadata(), &mut path, &mut origins);
}
@@ -192,16 +213,25 @@ impl ConfigLayerStack {
/// Returns the highest-precedence to lowest-precedence layers, so
/// `ConfigLayerSource::SessionFlags` would be first, if present.
pub fn layers_high_to_low(&self) -> Vec<&ConfigLayerEntry> {
self.get_layers(ConfigLayerStackOrdering::HighestPrecedenceFirst)
self.get_layers(ConfigLayerStackOrdering::HighestPrecedenceFirst, false)
}
/// Returns the highest-precedence to lowest-precedence layers, so
/// `ConfigLayerSource::SessionFlags` would be first, if present.
pub fn get_layers(&self, ordering: ConfigLayerStackOrdering) -> Vec<&ConfigLayerEntry> {
match ordering {
ConfigLayerStackOrdering::HighestPrecedenceFirst => self.layers.iter().rev().collect(),
ConfigLayerStackOrdering::LowestPrecedenceFirst => self.layers.iter().collect(),
pub fn get_layers(
&self,
ordering: ConfigLayerStackOrdering,
include_disabled: bool,
) -> Vec<&ConfigLayerEntry> {
let mut layers: Vec<&ConfigLayerEntry> = self
.layers
.iter()
.filter(|layer| include_disabled || !layer.is_disabled())
.collect();
if ordering == ConfigLayerStackOrdering::HighestPrecedenceFirst {
layers.reverse();
}
layers
}
}

View File

@@ -132,6 +132,7 @@ async fn returns_empty_when_all_layers_missing() {
},
config: TomlValue::Table(toml::map::Map::new()),
version: version_for_toml(&TomlValue::Table(toml::map::Map::new())),
disabled_reason: None,
},
user_layer,
);
@@ -546,6 +547,7 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s
},
config: TomlValue::Table(toml::map::Map::new()),
version: version_for_toml(&TomlValue::Table(toml::map::Map::new())),
disabled_reason: None,
}],
project_layers
);
@@ -554,7 +556,7 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s
}
#[tokio::test]
async fn project_layers_skipped_when_untrusted_or_unknown() -> std::io::Result<()> {
async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result<()> {
let tmp = tempdir()?;
let project_root = tmp.path().join("project");
let nested = project_root.join("child");
@@ -576,6 +578,13 @@ async fn project_layers_skipped_when_untrusted_or_unknown() -> std::io::Result<(
None,
)
.await?;
let untrusted_config_path = codex_home_untrusted.join(CONFIG_TOML_FILE);
let untrusted_config_contents = tokio::fs::read_to_string(&untrusted_config_path).await?;
tokio::fs::write(
&untrusted_config_path,
format!("foo = \"user\"\n{untrusted_config_contents}"),
)
.await?;
let layers_untrusted = load_config_layers_state(
&codex_home_untrusted,
@@ -584,16 +593,35 @@ async fn project_layers_skipped_when_untrusted_or_unknown() -> std::io::Result<(
LoaderOverrides::default(),
)
.await?;
let project_layers_untrusted = layers_untrusted
.layers_high_to_low()
let project_layers_untrusted: Vec<_> = layers_untrusted
.get_layers(
super::ConfigLayerStackOrdering::HighestPrecedenceFirst,
true,
)
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.count();
assert_eq!(project_layers_untrusted, 0);
assert_eq!(layers_untrusted.effective_config().get("foo"), None);
.collect();
assert_eq!(project_layers_untrusted.len(), 1);
assert!(
project_layers_untrusted[0].disabled_reason.is_some(),
"expected untrusted project layer to be disabled"
);
assert_eq!(
project_layers_untrusted[0].config.get("foo"),
Some(&TomlValue::String("child".to_string()))
);
assert_eq!(
layers_untrusted.effective_config().get("foo"),
Some(&TomlValue::String("user".to_string()))
);
let codex_home_unknown = tmp.path().join("home_unknown");
tokio::fs::create_dir_all(&codex_home_unknown).await?;
tokio::fs::write(
codex_home_unknown.join(CONFIG_TOML_FILE),
"foo = \"user\"\n",
)
.await?;
let layers_unknown = load_config_layers_state(
&codex_home_unknown,
@@ -602,13 +630,92 @@ async fn project_layers_skipped_when_untrusted_or_unknown() -> std::io::Result<(
LoaderOverrides::default(),
)
.await?;
let project_layers_unknown = layers_unknown
.layers_high_to_low()
let project_layers_unknown: Vec<_> = layers_unknown
.get_layers(
super::ConfigLayerStackOrdering::HighestPrecedenceFirst,
true,
)
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.count();
assert_eq!(project_layers_unknown, 0);
assert_eq!(layers_unknown.effective_config().get("foo"), None);
.collect();
assert_eq!(project_layers_unknown.len(), 1);
assert!(
project_layers_unknown[0].disabled_reason.is_some(),
"expected unknown-trust project layer to be disabled"
);
assert_eq!(
project_layers_unknown[0].config.get("foo"),
Some(&TomlValue::String("child".to_string()))
);
assert_eq!(
layers_unknown.effective_config().get("foo"),
Some(&TomlValue::String("user".to_string()))
);
Ok(())
}
#[tokio::test]
async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io::Result<()> {
let tmp = tempdir()?;
let project_root = tmp.path().join("project");
let nested = project_root.join("child");
tokio::fs::create_dir_all(nested.join(".codex")).await?;
tokio::fs::write(project_root.join(".git"), "gitdir: here").await?;
tokio::fs::write(nested.join(".codex").join(CONFIG_TOML_FILE), "foo =").await?;
let cwd = AbsolutePathBuf::from_absolute_path(&nested)?;
let cases = [
("untrusted", Some(TrustLevel::Untrusted)),
("unknown", None),
];
for (name, trust_level) in cases {
let codex_home = tmp.path().join(format!("home_{name}"));
tokio::fs::create_dir_all(&codex_home).await?;
let config_path = codex_home.join(CONFIG_TOML_FILE);
if let Some(trust_level) = trust_level {
make_config_for_test(&codex_home, &project_root, trust_level, None).await?;
let config_contents = tokio::fs::read_to_string(&config_path).await?;
tokio::fs::write(&config_path, format!("foo = \"user\"\n{config_contents}")).await?;
} else {
tokio::fs::write(&config_path, "foo = \"user\"\n").await?;
}
let layers = load_config_layers_state(
&codex_home,
Some(cwd.clone()),
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
)
.await?;
let project_layers: Vec<_> = layers
.get_layers(
super::ConfigLayerStackOrdering::HighestPrecedenceFirst,
true,
)
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.collect();
assert_eq!(
project_layers.len(),
1,
"expected one project layer for {name}"
);
assert!(
project_layers[0].disabled_reason.is_some(),
"expected {name} project layer to be disabled"
);
assert_eq!(
project_layers[0].config,
TomlValue::Table(toml::map::Map::new())
);
assert_eq!(
layers.effective_config().get("foo"),
Some(&TomlValue::String("user".to_string()))
);
}
Ok(())
}