Reject legacy [profiles] when using profile-v2 (#22647)

## Why `profile-v2` layers the selected profile file on top of the base user `config.toml`, but the legacy `[profiles]` table also stores named profile overrides in that same base file. Allowing both paths during one load makes it too easy to get a mixed profile where stale legacy settings still influence a profile-v2 run. ## What Changed - Detect a legacy `[profiles]` table in the base user config whenever `--profile-v2` selects a profile file. - Fail config loading with an `InvalidData` error that tells the user to move those settings into the selected profile-v2 file or remove `[profiles]`. - Add a loader regression covering `--profile-v2` with legacy `[profiles]` in `config.toml`. ## Testing - `cargo test -p codex-config profile_v2_rejects_legacy_profiles_in_base_user_config`
2026-07-01 00:31:56 +08:00 · 2026-05-15 11:35:42 +02:00
parent 302149d979
commit a5e5faf216
2 changed files with 202 additions and 12 deletions
@@ -1,6 +1,8 @@
 mod layer_io;
 #[cfg(target_os = "macos")]
 mod macos;
+#[cfg(test)]
+mod tests;

 use self::layer_io::LoadedConfigLayers;
 use crate::CONFIG_TOML_FILE;
@@ -211,19 +213,35 @@ pub async fn load_config_layers_state(
    // Add the base user config layer. When profile-v2 is selected, add the
    // profile config as a second user layer on top so the profile only needs to
    // contain overrides.
-    let base_user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home);
-    layers.push(
-        load_user_config_layer(
-            fs,
-            &base_user_file,
-            /*profile*/ None,
-            ignore_user_config,
-            strict_config,
-        )
-        .await?,
-    );
-
    let active_user_file = overrides.user_config_path(codex_home)?;
+    let base_user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home);
+    let base_user_layer = load_user_config_layer(
+        fs,
+        &base_user_file,
+        /*profile*/ None,
+        ignore_user_config,
+        strict_config,
+    )
+    .await?;
+    if let Some(active_user_profile) = active_user_profile.as_ref()
+        && base_user_layer.config.as_table().is_some_and(|config| {
+            config
+                .get("profiles")
+                .and_then(TomlValue::as_table)
+                .is_some_and(|profiles| profiles.contains_key(active_user_profile.as_str()))
+        })
+    {
+        return Err(io::Error::new(
+            io::ErrorKind::InvalidData,
+            format!(
+                "--profile-v2 `{active_user_profile}` cannot be used while {} contains legacy `[profiles.{active_user_profile}]` config; move those settings into {} or remove `[profiles.{active_user_profile}]`",
+                base_user_file.as_path().display(),
+                active_user_file.as_path().display()
+            ),
+        ));
+    }
+    layers.push(base_user_layer);
+
    if active_user_file != base_user_file {
        layers.push(
            load_user_config_layer(
@@ -0,0 +1,172 @@
+use super::*;
+use async_trait::async_trait;
+use codex_file_system::CopyOptions;
+use codex_file_system::CreateDirectoryOptions;
+use codex_file_system::FileMetadata;
+use codex_file_system::FileSystemResult;
+use codex_file_system::FileSystemSandboxContext;
+use codex_file_system::ReadDirectoryEntry;
+use codex_file_system::RemoveOptions;
+use pretty_assertions::assert_eq;
+use tempfile::tempdir;
+
+struct TestFileSystem;
+
+#[async_trait]
+impl ExecutorFileSystem for TestFileSystem {
+    async fn read_file(
+        &self,
+        path: &AbsolutePathBuf,
+        _sandbox: Option<&FileSystemSandboxContext>,
+    ) -> FileSystemResult<Vec<u8>> {
+        tokio::fs::read(path.as_path()).await
+    }
+
+    async fn write_file(
+        &self,
+        _path: &AbsolutePathBuf,
+        _contents: Vec<u8>,
+        _sandbox: Option<&FileSystemSandboxContext>,
+    ) -> FileSystemResult<()> {
+        unimplemented!("test filesystem only supports reads")
+    }
+
+    async fn create_directory(
+        &self,
+        _path: &AbsolutePathBuf,
+        _create_directory_options: CreateDirectoryOptions,
+        _sandbox: Option<&FileSystemSandboxContext>,
+    ) -> FileSystemResult<()> {
+        unimplemented!("test filesystem only supports reads")
+    }
+
+    async fn get_metadata(
+        &self,
+        _path: &AbsolutePathBuf,
+        _sandbox: Option<&FileSystemSandboxContext>,
+    ) -> FileSystemResult<FileMetadata> {
+        unimplemented!("test filesystem only supports reads")
+    }
+
+    async fn read_directory(
+        &self,
+        _path: &AbsolutePathBuf,
+        _sandbox: Option<&FileSystemSandboxContext>,
+    ) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
+        unimplemented!("test filesystem only supports reads")
+    }
+
+    async fn remove(
+        &self,
+        _path: &AbsolutePathBuf,
+        _remove_options: RemoveOptions,
+        _sandbox: Option<&FileSystemSandboxContext>,
+    ) -> FileSystemResult<()> {
+        unimplemented!("test filesystem only supports reads")
+    }
+
+    async fn copy(
+        &self,
+        _source_path: &AbsolutePathBuf,
+        _destination_path: &AbsolutePathBuf,
+        _copy_options: CopyOptions,
+        _sandbox: Option<&FileSystemSandboxContext>,
+    ) -> FileSystemResult<()> {
+        unimplemented!("test filesystem only supports reads")
+    }
+}
+
+#[tokio::test]
+async fn profile_v2_rejects_matching_legacy_profile_in_base_user_config() {
+    let tmp = tempdir().expect("tempdir");
+    let selected_config = tmp.path().join("work.config.toml");
+
+    std::fs::write(
+        tmp.path().join(CONFIG_TOML_FILE),
+        r#"
+model = "gpt-main"
+
+[profiles.work]
+model = "gpt-work"
+"#,
+    )
+    .expect("write default user config");
+    std::fs::write(&selected_config, r#"model = "gpt-work-v2""#)
+        .expect("write selected user config");
+
+    let mut overrides = LoaderOverrides::without_managed_config_for_tests();
+    overrides.user_config_path = Some(AbsolutePathBuf::resolve_path_against_base(
+        "work.config.toml",
+        tmp.path(),
+    ));
+    overrides.user_config_profile = Some("work".parse().expect("profile-v2 name"));
+
+    let err = load_config_layers_state(
+        &TestFileSystem,
+        tmp.path(),
+        /*cwd*/ None,
+        &[],
+        overrides,
+        CloudRequirementsLoader::default(),
+        &crate::NoopThreadConfigLoader,
+    )
+    .await
+    .expect_err("profile-v2 should reject a matching legacy profile in base user config");
+
+    assert_eq!(
+        err.kind(),
+        io::ErrorKind::InvalidData,
+        "a matching legacy profile should be a hard config error"
+    );
+    let message = err.to_string();
+    assert!(
+        message.contains("--profile-v2 `work` cannot be used"),
+        "unexpected error message: {message}"
+    );
+    assert!(
+        message.contains("config.toml"),
+        "unexpected error message: {message}"
+    );
+    assert!(
+        message.contains("[profiles.work]"),
+        "unexpected error message: {message}"
+    );
+}
+
+#[tokio::test]
+async fn profile_v2_allows_unrelated_legacy_profiles_in_base_user_config() {
+    let tmp = tempdir().expect("tempdir");
+    let selected_config = tmp.path().join("work.config.toml");
+
+    std::fs::write(
+        tmp.path().join(CONFIG_TOML_FILE),
+        r#"
+model = "gpt-main"
+
+[profiles.dev]
+model = "gpt-dev"
+"#,
+    )
+    .expect("write default user config");
+    std::fs::write(&selected_config, r#"model = "gpt-work-v2""#)
+        .expect("write selected user config");
+
+    let mut overrides = LoaderOverrides::without_managed_config_for_tests();
+    overrides.user_config_path = Some(AbsolutePathBuf::resolve_path_against_base(
+        "work.config.toml",
+        tmp.path(),
+    ));
+    overrides.user_config_profile = Some("work".parse().expect("profile-v2 name"));
+
+    load_config_layers_state(
+        &TestFileSystem,
+        tmp.path(),
+        /*cwd*/ None,
+        &[],
+        overrides,
+        CloudRequirementsLoader::default(),
+        &crate::NoopThreadConfigLoader,
+    )
+    .await
+    .expect("profile-v2 should allow unrelated legacy profiles in base user config");
+}