Skip to content

fix: ignore dangerous project-level config keys#20098

Merged
owenlin0 merged 2 commits into
mainfrom
owen/fix_project_profile_vuln
Apr 30, 2026
Merged

fix: ignore dangerous project-level config keys#20098
owenlin0 merged 2 commits into
mainfrom
owen/fix_project_profile_vuln

Conversation

@owenlin0

@owenlin0 owenlin0 commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Description

Ignore these top-level config keys when loading project-scoped config.toml files:

    "openai_base_url",
    "chatgpt_base_url",
    "model_provider",
    "model_providers",
    "notify",
    "profile",
    "profiles",
    "experimental_realtime_ws_base_url",

What changed

  • Add a project-local config denylist for credential-routing fields such as openai_base_url, chatgpt_base_url, model_provider, model_providers, profile, profiles, and experimental_realtime_ws_base_url.
  • Strip those fields from project config layers before they participate in effective config merging, while leaving safe project-local settings intact.
  • Track ignored project-local keys on config layers and surface a startup warning telling users to move those settings to user-level config.toml if they intentionally need them.
  • Update profile behavior coverage so project-local profile / profiles entries are ignored instead of overriding user-level profile selection.

Verification

  • cargo test -p codex-config
  • cargo test -p codex-core project_layer_ignores_unsupported_config_keys
  • cargo test -p codex-core project_profiles_are_ignored
  • cargo test -p codex-core config::config_loader_tests

@owenlin0 owenlin0 marked this pull request as ready for review April 28, 2026 23:20
@owenlin0 owenlin0 requested a review from a team as a code owner April 28, 2026 23:20
@owenlin0 owenlin0 requested a review from bolinfest April 28, 2026 23:21
Comment thread codex-rs/config/src/loader/mod.rs Outdated
Comment thread codex-rs/core/src/config/mod.rs Outdated
Comment thread codex-rs/config/src/loader/mod.rs
@owenlin0 owenlin0 force-pushed the owen/fix_project_profile_vuln branch from 726716f to f581d35 Compare April 30, 2026 18:32
@owenlin0 owenlin0 force-pushed the owen/fix_project_profile_vuln branch from 17bf1df to 5e21418 Compare April 30, 2026 18:59
@owenlin0 owenlin0 requested a review from bolinfest April 30, 2026 21:47
Comment thread codex-rs/config/src/state.rs Outdated
pub raw_toml: Option<String>,
pub version: String,
pub disabled_reason: Option<String>,
pub warnings: Vec<String>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this to Option<Vec<String>> so:

  • None means "not checked for warnings" (default)
  • Some(vec![]) means "checked and there were no warnings"
  • otherwise: checked and here are the warnings

Incidentally, when the type is Option<Vec<T>>, Some(vec![]) and vec![] would both take up three words because the first word is a pointer (which must be non-zero), so I believe the Option<Vec<String>> shouldn't take up more space than Vec<String> in this case.

Comment thread codex-rs/config/src/loader/mod.rs Outdated
} else {
ConfigLayerEntry::new(source, config)
};
entry.with_warnings(warnings)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, this feels better

Comment thread codex-rs/config/src/loader/mod.rs Outdated
let ignored_keys = ignored_keys.join(", ");
format!(
concat!(
"Ignored unsupported project-local config keys in {}: {}. ",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider

Suggested change
"Ignored unsupported project-local config keys in {}: {}. ",
"Ignored unsupported project-local config keys in {config_path}: {ignored_keys}. ",
...
config_path = config_path.display(),

@owenlin0 owenlin0 enabled auto-merge (squash) April 30, 2026 22:36
@owenlin0 owenlin0 merged commit 9ddb267 into main Apr 30, 2026
25 checks passed
@owenlin0 owenlin0 deleted the owen/fix_project_profile_vuln branch April 30, 2026 23:03
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants