Files
codex/codex-rs/config/src/state.rs
Michael Bolin 889ee018e7 config: add strict config parsing (#20559)
## Why

Codex intentionally ignores unknown `config.toml` fields by default so
older and newer config files keep working across versions. That leniency
also makes typo detection hard because misspelled or misplaced keys
disappear silently.

This change adds an opt-in strict config mode so users and tooling can
fail fast on unrecognized config fields without changing the default
permissive behavior.

This feature is possible because `serde_ignored` exposes the exact
signal Codex needs: it lets Codex run ordinary Serde deserialization
while recording fields Serde would otherwise ignore. That avoids
requiring `#[serde(deny_unknown_fields)]` across every config type and
keeps strict validation opt-in around the existing config model.

## What Changed

### Added strict config validation

- Added `serde_ignored`-based validation for `ConfigToml` in
`codex-rs/config/src/strict_config.rs`.
- Combined `serde_ignored` with `serde_path_to_error` so strict mode
preserves typed config error paths while also collecting fields Serde
would otherwise ignore.
- Added strict-mode validation for unknown `[features]` keys, including
keys that would otherwise be accepted by `FeaturesToml`'s flattened
boolean map.
- Kept typed config errors ahead of ignored-field reporting, so
malformed known fields are reported before unknown-field diagnostics.
- Added source-range diagnostics for top-level and nested unknown config
fields, including non-file managed preference source names.

### Kept parsing single-pass per source

- Reworked file and managed-config loading so strict validation reuses
the already parsed `TomlValue` for that source.
- For actual config files and managed config strings, the loader now
reads once, parses once, and validates that same parsed value instead of
deserializing multiple times.
- Validated `-c` / `--config` override layers with the same
base-directory context used for normal relative-path resolution, so
unknown override keys are still reported when another override contains
a relative path.

### Scoped `--strict-config` to config-heavy entry points

- Added support for `--strict-config` on the main config-loading entry
points where it is most useful:
  - `codex`
  - `codex resume`
  - `codex fork`
  - `codex exec`
  - `codex review`
  - `codex mcp-server`
  - `codex app-server` when running the server itself
  - the standalone `codex-app-server` binary
  - the standalone `codex-exec` binary
- Commands outside that set now reject `--strict-config` early with
targeted errors instead of accepting it everywhere through shared CLI
plumbing.
- `codex app-server` subcommands such as `proxy`, `daemon`, and
`generate-*` are intentionally excluded from the first rollout.
- When app-server strict mode sees invalid config, app-server exits with
the config error instead of logging a warning and continuing with
defaults.
- Introduced a dedicated `ReviewCommand` wrapper in `codex-rs/cli`
instead of extending shared `ReviewArgs`, so `--strict-config` stays on
the outer config-loading command surface and does not become part of the
reusable review payload used by `codex exec review`.

### Coverage

- Added tests for top-level and nested unknown config fields, unknown
`[features]` keys, typed-error precedence, source-location reporting,
and non-file managed preference source names.
- Added CLI coverage showing invalid `--enable`, invalid `--disable`,
and unknown `-c` overrides still error when `--strict-config` is
present, including compound-looking feature names such as
`multi_agent_v2.subagent_usage_hint_text`.
- Added integration coverage showing both `codex app-server
--strict-config` and standalone `codex-app-server --strict-config` exit
with an error for unknown config fields instead of starting with
fallback defaults.
- Added coverage showing unsupported command surfaces reject
`--strict-config` with explicit errors.

## Example Usage

Run Codex with strict config validation enabled:

```shell
codex --strict-config
```

Strict config mode is also available on the supported config-heavy
subcommands:

```shell
codex --strict-config exec "explain this repository"
codex review --strict-config --uncommitted
codex mcp-server --strict-config
codex app-server --strict-config --listen off
codex-app-server --strict-config --listen off
```

For example, if `~/.codex/config.toml` contains a typo in a key name:

```toml
model = "gpt-5"
approval_polic = "on-request"
```

then `codex --strict-config` reports the misspelled key instead of
silently ignoring it. The path is shortened to `~` here for readability:

```text
$ codex --strict-config
Error loading config.toml:
~/.codex/config.toml:2:1: unknown configuration field `approval_polic`
  |
2 | approval_polic = "on-request"
  | ^^^^^^^^^^^^^^
```

Without `--strict-config`, Codex keeps the existing permissive behavior
and ignores the unknown key.

Strict config mode also validates ad-hoc `-c` / `--config` overrides:

```text
$ codex --strict-config -c foo=bar
Error: unknown configuration field `foo` in -c/--config override

$ codex --strict-config -c features.foo=true
Error: unknown configuration field `features.foo` in -c/--config override
```

Invalid feature toggles are rejected too, including values that look
like nested config paths:

```text
$ codex --strict-config --enable does_not_exist
Error: Unknown feature flag: does_not_exist

$ codex --strict-config --disable does_not_exist
Error: Unknown feature flag: does_not_exist

$ codex --strict-config --enable multi_agent_v2.subagent_usage_hint_text
Error: Unknown feature flag: multi_agent_v2.subagent_usage_hint_text
```

Unsupported commands reject the flag explicitly:

```text
$ codex --strict-config cloud list
Error: `--strict-config` is not supported for `codex cloud`
```

## Verification

The `codex-cli` `strict_config` tests cover invalid `--enable`, invalid
`--disable`, the compound `multi_agent_v2.subagent_usage_hint_text`
case, unknown `-c` overrides, app-server strict startup failure through
`codex app-server`, and rejection for unsupported commands such as
`codex cloud`, `codex mcp`, `codex remote-control`, and `codex
app-server proxy`.

The config and config-loader tests cover unknown top-level fields,
unknown nested fields, unknown `[features]` keys, source-location
reporting, non-file managed config sources, and `-c` validation for keys
such as `features.foo`.

The app-server test suite covers standalone `codex-app-server
--strict-config` startup failure for an unknown config field.

## Documentation

The Codex CLI docs on developers.openai.com/codex should mention
`--strict-config` as an opt-in validation mode for supported
config-heavy entry points once this ships.
2026-05-13 16:08:05 +00:00

457 lines
16 KiB
Rust

use crate::config_requirements::ConfigRequirements;
use crate::config_requirements::ConfigRequirementsToml;
use super::fingerprint::record_origins;
use super::fingerprint::version_for_toml;
use super::key_aliases::normalized_with_key_aliases;
use super::merge::merge_toml_values;
use codex_app_server_protocol::ConfigLayer;
use codex_app_server_protocol::ConfigLayerMetadata;
use codex_app_server_protocol::ConfigLayerSource;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde_json::Value as JsonValue;
use std::collections::HashMap;
use std::path::PathBuf;
use toml::Value as TomlValue;
/// User-facing config loading behavior that is not part of the config document.
#[derive(Debug, Default, Clone)]
pub struct ConfigLoadOptions {
pub loader_overrides: LoaderOverrides,
pub strict_config: bool,
}
impl From<LoaderOverrides> for ConfigLoadOptions {
fn from(loader_overrides: LoaderOverrides) -> Self {
Self {
loader_overrides,
strict_config: false,
}
}
}
/// LoaderOverrides overrides managed configuration inputs (primarily for tests).
#[derive(Debug, Default, Clone)]
pub struct LoaderOverrides {
pub managed_config_path: Option<PathBuf>,
pub system_config_path: Option<PathBuf>,
pub system_requirements_path: Option<PathBuf>,
pub ignore_managed_requirements: bool,
pub ignore_user_config: bool,
pub ignore_user_and_project_exec_policy_rules: bool,
//TODO(gt): Add a macos_ prefix to this field and remove the target_os check.
#[cfg(target_os = "macos")]
pub managed_preferences_base64: Option<String>,
pub macos_managed_config_requirements_base64: Option<String>,
}
impl LoaderOverrides {
/// Returns overrides that ignore host-managed configuration.
///
/// This is intended for tests that should load only repo-controlled config fixtures.
pub fn without_managed_config_for_tests() -> Self {
let base = std::env::temp_dir().join("codex-config-tests");
Self {
managed_config_path: Some(base.join("managed_config.toml")),
system_config_path: Some(base.join("config.toml")),
system_requirements_path: Some(base.join("requirements.toml")),
ignore_managed_requirements: false,
ignore_user_config: false,
ignore_user_and_project_exec_policy_rules: false,
#[cfg(target_os = "macos")]
managed_preferences_base64: Some(String::new()),
macos_managed_config_requirements_base64: Some(String::new()),
}
}
/// Returns overrides with host MDM disabled and managed config loaded from `managed_config_path`.
///
/// This is intended for tests that supply an explicit managed config fixture.
pub fn with_managed_config_path_for_tests(managed_config_path: PathBuf) -> Self {
Self {
managed_config_path: Some(managed_config_path),
..Self::without_managed_config_for_tests()
}
}
}
#[derive(Debug, Clone, PartialEq)]
pub struct ConfigLayerEntry {
pub name: ConfigLayerSource,
pub config: TomlValue,
pub raw_toml: Option<String>,
pub version: String,
pub disabled_reason: Option<String>,
hooks_config_folder_override: Option<AbsolutePathBuf>,
}
impl ConfigLayerEntry {
pub fn new(name: ConfigLayerSource, config: TomlValue) -> Self {
let version = version_for_toml(&config);
Self {
name,
config,
raw_toml: None,
version,
disabled_reason: None,
hooks_config_folder_override: None,
}
}
pub fn new_with_raw_toml(name: ConfigLayerSource, config: TomlValue, raw_toml: String) -> Self {
let version = version_for_toml(&config);
Self {
name,
config,
raw_toml: Some(raw_toml),
version,
disabled_reason: None,
hooks_config_folder_override: None,
}
}
pub fn new_disabled(
name: ConfigLayerSource,
config: TomlValue,
disabled_reason: impl Into<String>,
) -> Self {
let version = version_for_toml(&config);
Self {
name,
config,
raw_toml: None,
version,
disabled_reason: Some(disabled_reason.into()),
hooks_config_folder_override: None,
}
}
pub fn is_disabled(&self) -> bool {
self.disabled_reason.is_some()
}
pub fn raw_toml(&self) -> Option<&str> {
self.raw_toml.as_deref()
}
pub(crate) fn with_hooks_config_folder_override(
mut self,
hooks_config_folder_override: Option<AbsolutePathBuf>,
) -> Self {
self.hooks_config_folder_override = hooks_config_folder_override;
self
}
pub fn metadata(&self) -> ConfigLayerMetadata {
ConfigLayerMetadata {
name: self.name.clone(),
version: self.version.clone(),
}
}
pub fn as_layer(&self) -> ConfigLayer {
ConfigLayer {
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(),
}
}
// Get the `.codex/` folder associated with this config layer, if any.
pub fn config_folder(&self) -> Option<AbsolutePathBuf> {
match &self.name {
ConfigLayerSource::Mdm { .. } => None,
ConfigLayerSource::System { file } => file.parent(),
ConfigLayerSource::User { file } => file.parent(),
ConfigLayerSource::Project { dot_codex_folder } => Some(dot_codex_folder.clone()),
ConfigLayerSource::SessionFlags => None,
ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } => None,
ConfigLayerSource::LegacyManagedConfigTomlFromMdm => None,
}
}
/// Returns the `.codex/` folder that should be used for hook declarations.
///
/// Project layers normally use their own config folder. Linked Git worktrees
/// can instead point hook discovery at the matching folder from the root
/// checkout while the rest of the project config still comes from the
/// worktree.
pub fn hooks_config_folder(&self) -> Option<AbsolutePathBuf> {
self.hooks_config_folder_override
.clone()
.or_else(|| self.config_folder())
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ConfigLayerStackOrdering {
LowestPrecedenceFirst,
HighestPrecedenceFirst,
}
#[derive(Debug, Clone, Default, PartialEq)]
pub struct ConfigLayerStack {
/// Layers are listed from lowest precedence (base) to highest (top), so
/// later entries in the Vec override earlier ones.
layers: Vec<ConfigLayerEntry>,
/// Index into [layers] of the user config layer, if any.
user_layer_index: Option<usize>,
/// Constraints that must be enforced when deriving a [Config] from the
/// layers.
requirements: ConfigRequirements,
/// Raw requirements data as loaded from requirements.toml/MDM/legacy
/// sources. This preserves the original allow-lists so they can be
/// surfaced via APIs.
requirements_toml: ConfigRequirementsToml,
/// Whether execpolicy should skip `.rules` files from user and project config-layer folders.
ignore_user_and_project_exec_policy_rules: bool,
/// Startup warnings discovered while building this stack.
///
/// `None` means the loader did not check for stack-level warnings, while
/// `Some(vec![])` means it checked and found nothing to report.
startup_warnings: Option<Vec<String>>,
}
impl ConfigLayerStack {
pub fn new(
layers: Vec<ConfigLayerEntry>,
requirements: ConfigRequirements,
requirements_toml: ConfigRequirementsToml,
) -> std::io::Result<Self> {
let user_layer_index = verify_layer_ordering(&layers)?;
Ok(Self {
layers,
user_layer_index,
requirements,
requirements_toml,
ignore_user_and_project_exec_policy_rules: false,
startup_warnings: None,
})
}
pub fn with_user_and_project_exec_policy_rules_ignored(
mut self,
ignore_user_and_project_exec_policy_rules: bool,
) -> Self {
self.ignore_user_and_project_exec_policy_rules = ignore_user_and_project_exec_policy_rules;
self
}
pub fn ignore_user_and_project_exec_policy_rules(&self) -> bool {
self.ignore_user_and_project_exec_policy_rules
}
pub(crate) fn with_startup_warnings(mut self, startup_warnings: Vec<String>) -> Self {
self.startup_warnings = Some(startup_warnings);
self
}
pub fn startup_warnings(&self) -> Option<&[String]> {
self.startup_warnings.as_deref()
}
/// Returns the raw user config layer, if any.
///
/// This does not merge other config layers or apply any requirements.
pub fn get_user_layer(&self) -> Option<&ConfigLayerEntry> {
self.user_layer_index
.and_then(|index| self.layers.get(index))
}
pub fn requirements(&self) -> &ConfigRequirements {
&self.requirements
}
pub fn requirements_toml(&self) -> &ConfigRequirementsToml {
&self.requirements_toml
}
/// Creates a new [ConfigLayerStack] using the specified values to inject a
/// "user layer" into the stack. If such a layer already exists, it is
/// replaced; otherwise, it is inserted into the stack at the appropriate
/// position based on precedence rules.
pub fn with_user_config(&self, config_toml: &AbsolutePathBuf, user_config: TomlValue) -> Self {
self.with_user_layer(Some(ConfigLayerEntry::new(
ConfigLayerSource::User {
file: config_toml.clone(),
},
user_config,
)))
}
/// Returns a new stack with the user layer copied from `other`, preserving
/// every non-user layer already present in this stack.
pub fn with_user_layer_from(&self, other: &Self) -> Self {
self.with_user_layer(other.get_user_layer().cloned())
}
fn with_user_layer(&self, user_layer: Option<ConfigLayerEntry>) -> Self {
let mut layers = self.layers.clone();
let user_layer_index = match (self.user_layer_index, user_layer) {
(Some(index), Some(user_layer)) => {
layers[index] = user_layer;
Some(index)
}
(Some(index), None) => {
layers.remove(index);
None
}
(None, Some(user_layer)) => {
let user_layer_index = match layers
.iter()
.position(|layer| layer.name.precedence() > user_layer.name.precedence())
{
Some(index) => {
layers.insert(index, user_layer);
index
}
None => {
layers.push(user_layer);
layers.len() - 1
}
};
Some(user_layer_index)
}
(None, None) => None,
};
Self {
layers,
user_layer_index,
requirements: self.requirements.clone(),
requirements_toml: self.requirements_toml.clone(),
ignore_user_and_project_exec_policy_rules: self
.ignore_user_and_project_exec_policy_rules,
startup_warnings: self.startup_warnings.clone(),
}
}
/// Returns the merged config-layer view.
///
/// This only merges ordinary config layers and does not apply requirements
/// such as cloud requirements.
pub fn effective_config(&self) -> TomlValue {
let mut merged = TomlValue::Table(toml::map::Map::new());
for layer in self.get_layers(
ConfigLayerStackOrdering::LowestPrecedenceFirst,
/*include_disabled*/ false,
) {
merge_toml_values(&mut merged, &layer.config);
}
merged
}
/// Returns field origins for the merged config-layer view.
///
/// Requirement sources are tracked separately and are not included here.
pub fn origins(&self) -> HashMap<String, ConfigLayerMetadata> {
let mut origins = HashMap::new();
let mut path = Vec::new();
for layer in self.get_layers(
ConfigLayerStackOrdering::LowestPrecedenceFirst,
/*include_disabled*/ false,
) {
let config = normalized_with_key_aliases(&layer.config, &[]);
record_origins(&config, &layer.metadata(), &mut path, &mut origins);
}
origins
}
/// Returns config layers from highest precedence to lowest precedence.
///
/// Requirement sources are tracked separately and are not included here.
pub fn layers_high_to_low(&self) -> Vec<&ConfigLayerEntry> {
self.get_layers(
ConfigLayerStackOrdering::HighestPrecedenceFirst,
/*include_disabled*/ false,
)
}
/// Returns config layers in the requested precedence order.
///
/// Requirement sources are tracked separately and are not included here.
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
}
}
/// Ensures precedence ordering of config layers is correct. Returns the index
/// of the user config layer, if any (at most one should exist).
fn verify_layer_ordering(layers: &[ConfigLayerEntry]) -> std::io::Result<Option<usize>> {
if !layers.iter().map(|layer| &layer.name).is_sorted() {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"config layers are not in correct precedence order",
));
}
// The previous check ensured `layers` is sorted by precedence, so now we
// further verify that:
// 1. There is at most one user config layer.
// 2. Project layers are ordered from root to cwd.
let mut user_layer_index: Option<usize> = None;
let mut previous_project_dot_codex_folder: Option<&AbsolutePathBuf> = None;
for (index, layer) in layers.iter().enumerate() {
if matches!(layer.name, ConfigLayerSource::User { .. }) {
if user_layer_index.is_some() {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"multiple user config layers found",
));
}
user_layer_index = Some(index);
}
if let ConfigLayerSource::Project {
dot_codex_folder: current_project_dot_codex_folder,
} = &layer.name
{
if let Some(previous) = previous_project_dot_codex_folder {
let Some(parent) = previous.as_path().parent() else {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"project layer has no parent directory",
));
};
if previous == current_project_dot_codex_folder
|| !current_project_dot_codex_folder
.as_path()
.ancestors()
.any(|ancestor| ancestor == parent)
{
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"project layers are not ordered from root to cwd",
));
}
}
previous_project_dot_codex_folder = Some(current_project_dot_codex_folder);
}
}
Ok(user_layer_index)
}
#[cfg(test)]
#[path = "state_tests.rs"]
mod tests;