Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions src-tauri/src/codex_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,37 @@ fn set_codex_model_catalog_json_field(
Ok(doc.to_string())
}

/// Strip provider-owned fields from a Codex `config.toml` text so that a live
/// config used to seed a failover snapshot cannot contaminate another provider.
///
/// Removed keys: `model`, `model_provider`, `base_url`, `model_providers`,
/// `experimental_bearer_token`, and `model_catalog_json`. These are the
/// active provider's routing, selected model, credentials, and catalog
/// reference; each failover target brings its own values via its snapshot, so
/// the live config must only contribute user-level customizations (e.g.
/// `model_reasoning_effort`, `disable_response_storage`, `[projects.*]`).
///
/// Note: `experimental_bearer_token` carries the active provider's API key when
/// `preserve_codex_official_auth_on_switch` is set, so leaving it in would leak
/// one provider's credential into another provider's failover snapshot.
pub fn strip_codex_provider_routing(config_text: &str) -> String {
let Ok(mut doc) = config_text.parse::<DocumentMut>() else {
return config_text.to_string();
};
let root = doc.as_table_mut();
for key in [
"model",
"model_provider",
"base_url",
"model_providers",
"experimental_bearer_token",
"model_catalog_json",
] {
root.remove(key);
}
doc.to_string()
}

#[derive(Clone, Debug)]
pub struct PreparedCodexConfigText {
pub config_text: String,
Expand Down Expand Up @@ -1345,6 +1376,53 @@ mod tests {
use std::ffi::OsString;
use tempfile::TempDir;

#[test]
fn strip_codex_provider_routing_removes_routing_and_keeps_other_keys() {
let input = "\nmodel = \"gpt-4o\"\nmodel_provider = \"alpha\"\nbase_url = \"https://legacy.example/v1\"\nexperimental_bearer_token = \"sk-alpha-secret\"\nmodel_catalog_json = \"cc-switch-model-catalog.json\"\nmodel_reasoning_effort = \"high\"\n\n[model_providers.alpha]\nbase_url = \"https://alpha.example/v1\"\nwire_api = \"chat\"\n\n[projects.foo]\nmodel = \"gpt-4o\"\n";
let stripped = strip_codex_provider_routing(input);

assert!(
!stripped.contains("model_provider"),
"model_provider removed: {stripped}"
);
assert!(
!stripped.contains("model_providers"),
"model_providers table removed: {stripped}"
);
assert!(
!stripped.contains("alpha"),
"no alpha routing leaked: {stripped}"
);
assert!(
!stripped.contains("sk-alpha-secret"),
"experimental_bearer_token (credential) removed: {stripped}"
);
assert!(
!stripped.contains("model_catalog_json"),
"model_catalog_json removed: {stripped}"
);
assert!(
!stripped.contains("base_url"),
"top-level base_url removed: {stripped}"
);
assert!(
stripped.contains("model_reasoning_effort"),
"non-routing top-level key preserved: {stripped}"
);
assert!(
stripped.contains("[projects.foo]"),
"non-routing table preserved: {stripped}"
);
}

#[test]
fn strip_codex_provider_routing_handles_empty_and_invalid_toml() {
assert_eq!(strip_codex_provider_routing(""), "");
let invalid = "this is not = = valid toml }}}";
let result = strip_codex_provider_routing(invalid);
assert_eq!(result, invalid, "invalid toml returned unchanged");
}

struct CodexHomeEnvGuard {
original: Option<OsString>,
}
Expand Down
9 changes: 9 additions & 0 deletions src-tauri/src/services/provider/codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,15 @@ impl ProviderService {
raw_settings.insert("auth".to_string(), auth);
}
raw_settings.insert("config".to_string(), Value::String(text));
// Preserve the live model catalog so it round-trips across switches.
// Without this the outgoing provider's snapshot loses `modelCatalog`,
// and switching back to it strips `model_catalog_json` from config.toml
// (effectively disabling the catalog). Mirrors read_codex_live_settings_with_model_catalog.
if let Ok(Some(model_catalog)) =
crate::codex_config::read_codex_model_catalog_simplified_from_live()
{
raw_settings.insert("modelCatalog".to_string(), model_catalog);
}
snapshot_provider.settings_config = Value::Object(raw_settings);
snapshot_provider = Self::migrate_provider_snapshot_for_storage(
&AppType::Codex,
Expand Down
68 changes: 68 additions & 0 deletions src-tauri/src/services/provider/codex_openai_auth_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,71 @@ fn migrate_legacy_codex_config_preserves_extra_keys() {
"should preserve disable_response_storage: {result}"
);
}

#[test]
#[serial]
fn codex_model_catalog_survives_round_trip_switch() {
let temp_home = TempDir::new().expect("create temp home");
let _env = TestEnvGuard::isolated(temp_home.path());
std::fs::create_dir_all(crate::codex_config::get_codex_config_dir())
.expect("create ~/.codex (initialized)");

let catalog = json!({
"models": [{ "model": "gpt-4o", "displayName": "GPT-4o" }]
});

let mut config = MultiAppConfig::default();
config.ensure_app(&AppType::Codex);
{
let manager = config
.get_manager_mut(&AppType::Codex)
.expect("codex manager");
manager.providers.insert(
"alpha".to_string(),
Provider::with_id(
"alpha".to_string(),
"Alpha".to_string(),
json!({
"auth": { "OPENAI_API_KEY": "sk-alpha" },
"config": "model_provider = \"alpha\"\nmodel = \"gpt-4o\"\n\n[model_providers.alpha]\nbase_url = \"https://alpha.example/v1\"\nwire_api = \"chat\"\n",
"modelCatalog": catalog,
}),
None,
),
);
manager.providers.insert(
"beta".to_string(),
Provider::with_id(
"beta".to_string(),
"Beta".to_string(),
json!({
"auth": { "OPENAI_API_KEY": "sk-beta" },
"config": "model_provider = \"beta\"\nmodel = \"gpt-4o\"\n\n[model_providers.beta]\nbase_url = \"https://beta.example/v1\"\nwire_api = \"chat\"\n",
}),
None,
),
);
}

let state = state_from_config(config);

// Switch to alpha: config.toml should install the model_catalog_json reference.
ProviderService::switch(&state, AppType::Codex, "alpha").expect("switch to alpha");
let after_alpha =
std::fs::read_to_string(get_codex_config_path()).expect("read config after alpha");
assert!(
after_alpha.contains("model_catalog_json"),
"alpha switch should install model_catalog_json: {after_alpha}"
);

// Round-trip: away to beta, then back to alpha.
ProviderService::switch(&state, AppType::Codex, "beta").expect("switch to beta");
ProviderService::switch(&state, AppType::Codex, "alpha").expect("switch back to alpha");

let after_round_trip =
std::fs::read_to_string(get_codex_config_path()).expect("read config after round trip");
assert!(
after_round_trip.contains("model_catalog_json"),
"model_catalog_json must survive a round-trip switch: {after_round_trip}"
);
}
147 changes: 146 additions & 1 deletion src-tauri/src/services/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,7 +1597,37 @@ impl ProxyService {
provider,
&mut provider_snapshot,
)?;
Self::merge_live_backup_snapshot(app_type, Some(original_live), None, provider_snapshot)

// For Codex, the user's live config carries the currently-active
// provider's routing (`model_provider` + `[model_providers.*]`). Strip it
// before the 2-way merge so it cannot contaminate another provider's
// failover snapshot; each target brings its own routing via
// `provider_snapshot`. Without this a failover to the official provider
// inherits the previous third-party `model_provider` and keeps routing to
// the old endpoint.
let stripped;
let existing: &Value = if matches!(app_type, AppType::Codex) {
stripped = Self::codex_live_backup_without_routing(original_live);
&stripped
} else {
original_live
};

Self::merge_live_backup_snapshot(app_type, Some(existing), None, provider_snapshot)
}

/// Return a copy of a Codex live backup with provider-routing fields removed
/// from its embedded `config` text. Non-routing keys are left untouched.
fn codex_live_backup_without_routing(original_live: &Value) -> Value {
let Some(config_text) = original_live.get("config").and_then(Value::as_str) else {
return original_live.clone();
};
let stripped = crate::codex_config::strip_codex_provider_routing(config_text);
let mut clone = original_live.clone();
if let Some(obj) = clone.as_object_mut() {
obj.insert("config".to_string(), Value::String(stripped));
}
clone
}

async fn save_failover_live_snapshot(
Expand Down Expand Up @@ -4022,6 +4052,121 @@ mod tests {
assert_eq!(env.get(key).and_then(Value::as_str), expected, "{key}");
}

#[test]
fn codex_failover_snapshot_excludes_foreign_provider_routing() {
let db = Arc::new(Database::memory().expect("create database"));
let service = ProxyService::new(db);

// The currently-live provider (alpha) carries its own routing in config.toml.
let original_live = json!({
"config": "\nmodel_provider = \"alpha\"\n\n[model_providers.alpha]\nbase_url = \"https://alpha.example/v1\"\nwire_api = \"chat\"\n",
"auth": {}
});
// The failover target (beta) has its own routing.
let provider_beta = Provider::with_id(
"beta".to_string(),
"Beta".to_string(),
json!({
"auth": {},
"config": "\nmodel_provider = \"beta\"\n\n[model_providers.beta]\nbase_url = \"https://beta.example/v1\"\nwire_api = \"chat\"\n",
}),
None,
);

let merged = service
.build_failover_live_snapshot(&AppType::Codex, &original_live, &provider_beta)
.expect("build beta failover snapshot");

let cfg = merged
.get("config")
.and_then(Value::as_str)
.expect("merged config text");
assert!(
cfg.contains("model_providers.beta"),
"beta's own routing must be present: {cfg}"
);
assert!(
!cfg.contains("alpha"),
"beta's failover snapshot leaked alpha's routing: {cfg}"
);
}

#[test]
fn codex_failover_snapshot_to_official_drops_third_party_routing() {
let db = Arc::new(Database::memory().expect("create database"));
let service = ProxyService::new(db);

// Currently-live third-party provider carries its own routing.
let original_live = json!({
"config": "\nmodel_provider = \"alpha\"\n\n[model_providers.alpha]\nbase_url = \"https://alpha.example/v1\"\nwire_api = \"chat\"\n",
"auth": {}
});
// The failover target is the official provider, which carries NO routing
// (no `model_provider`, no `[model_providers]` section) in config.toml.
let official = Provider::with_id(
"codex-official".to_string(),
"Codex Official".to_string(),
json!({
"auth": { "OPENAI_API_KEY": "official-key" },
"config": "\n# official codex: no model_provider routing\n",
}),
None,
);

let merged = service
.build_failover_live_snapshot(&AppType::Codex, &original_live, &official)
.expect("build official failover snapshot");

let cfg = merged
.get("config")
.and_then(Value::as_str)
.expect("merged config text");
assert!(
!cfg.contains("model_provider"),
"official failover snapshot must not keep third-party model_provider routing: {cfg}"
);
assert!(
!cfg.contains("alpha"),
"official failover snapshot leaked alpha's routing: {cfg}"
);
}

#[test]
fn codex_failover_snapshot_does_not_leak_bearer_token() {
// With preserve_codex_official_auth_on_switch the active provider's API
// key rides in config.toml as experimental_bearer_token. It must not leak
// into another provider's failover snapshot.
let db = Arc::new(Database::memory().expect("create database"));
let service = ProxyService::new(db);

let original_live = json!({
"config": "\nmodel_provider = \"alpha\"\nexperimental_bearer_token = \"sk-alpha-secret\"\n\n[model_providers.alpha]\nbase_url = \"https://alpha.example/v1\"\nwire_api = \"chat\"\n",
"auth": {}
});
let provider_beta = Provider::with_id(
"beta".to_string(),
"Beta".to_string(),
json!({
"auth": { "OPENAI_API_KEY": "sk-beta" },
"config": "\nmodel_provider = \"beta\"\n\n[model_providers.beta]\nbase_url = \"https://beta.example/v1\"\nwire_api = \"chat\"\n",
}),
None,
);

let merged = service
.build_failover_live_snapshot(&AppType::Codex, &original_live, &provider_beta)
.expect("build beta failover snapshot");

let cfg = merged
.get("config")
.and_then(Value::as_str)
.expect("merged config text");
assert!(
!cfg.contains("sk-alpha-secret"),
"beta's failover snapshot leaked alpha's bearer token: {cfg}"
);
}

#[test]
fn managed_account_claude_takeover_uses_api_key_placeholder() {
let mut provider = Provider::with_id(
Expand Down
Loading