mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Handle credential provider source remapping across config layers (#44241)
## What changed - Let higher-priority providers in `features.network_proxy.credentials` displace lower-priority providers with overlapping `env` sources, including in profiles. Preserve inherited settings when providers swap sources, and compare environment names case-insensitively on Windows. - Preserve provider definitions across ordered batch remaps and persist displaced-provider removals without overwriting unrelated sibling updates. - Validate remapped providers using the credential broker's compilation rules before persisting writes, while allowing incomplete drafts and explicit deletions. - Omit displaced providers from config origins and report writes overridden by another provider's source ownership. - Build trusted credential-broker configuration before merging managed layers for project discovery, preserving remapped providers' environment protections. ## Testing Add unit and config RPC regression tests for source precedence, source swaps, ordered edits, concurrent sibling updates, atomic rejection of invalid remaps, persistence, and override reporting. Add a macOS managed-config regression test for remapped provider bindings during project discovery. GitOrigin-RevId: b7d402727acf4e5b0f25946db1cf18811f5e2679
This commit is contained in:
1 parent
3436cad5ab
commit
ed4ca07ba6
14 files changed
+1690
-45
No files matched your search
@@ -1546,6 +1546,423 @@ model = "gpt-old"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_provider_write_reports_source_displacement() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
write_config(&codex_home, "")?;
|
||||
let mut app_server = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_auto_env()
|
||||
.with_args(&[
|
||||
"-c",
|
||||
"features.network_proxy.credentials.enforced.env=['VENDOR_PASSWORD']",
|
||||
"-c",
|
||||
"features.network_proxy.credentials.enforced.patterns=['^pin_[a-z]{8}$']",
|
||||
"-c",
|
||||
"features.network_proxy.credentials.enforced.url_prefixes=['https://fixed.example']",
|
||||
])
|
||||
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
|
||||
.await?;
|
||||
|
||||
let mut provider = json!({
|
||||
"env": ["VENDOR_PASSWORD"],
|
||||
"patterns": ["^pin_[a-z]{8}$"],
|
||||
"url_prefixes": ["https://user.example"],
|
||||
"auth": ["bearer"],
|
||||
});
|
||||
for (suffix, value, overridden) in [
|
||||
("", provider.clone(), true),
|
||||
(".url_prefixes", json!(["https://updated.example"]), true),
|
||||
(".env", json!(["OTHER_PASSWORD"]), false),
|
||||
] {
|
||||
let write_id = app_server
|
||||
.send_config_value_write_request(ConfigValueWriteParams {
|
||||
file_path: None,
|
||||
key_path: format!("features.network_proxy.credentials.user_provider{suffix}"),
|
||||
value: value.clone(),
|
||||
merge_strategy: MergeStrategy::Upsert,
|
||||
expected_version: None,
|
||||
})
|
||||
.await?;
|
||||
let write: ConfigWriteResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(write_id)).await??;
|
||||
let expected = overridden.then_some((ConfigLayerSource::SessionFlags, json!(null)));
|
||||
assert_eq!(
|
||||
(
|
||||
write.status,
|
||||
write
|
||||
.overridden_metadata
|
||||
.map(|meta| { (meta.overriding_layer.name, meta.effective_value) })
|
||||
),
|
||||
(
|
||||
if overridden {
|
||||
WriteStatus::OkOverridden
|
||||
} else {
|
||||
WriteStatus::Ok
|
||||
},
|
||||
expected,
|
||||
),
|
||||
);
|
||||
|
||||
if let Some(field) = suffix.strip_prefix('.') {
|
||||
provider[field] = value;
|
||||
}
|
||||
let saved: toml::Value = toml::from_str(&std::fs::read_to_string(
|
||||
codex_home.path().join("config.toml"),
|
||||
)?)?;
|
||||
assert_eq!(
|
||||
serde_json::to_value(
|
||||
&saved["features"]["network_proxy"]["credentials"]["user_provider"]
|
||||
)?,
|
||||
provider,
|
||||
);
|
||||
let read_id = app_server
|
||||
.send_config_read_request(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let read: ConfigReadResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(read_id)).await??;
|
||||
assert_eq!(
|
||||
read.config.additional["features"]["network_proxy"]["credentials"].get("user_provider"),
|
||||
(!overridden).then_some(&provider),
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_provider_field_removal_uses_higher_same_id_definition() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
write_config(
|
||||
&codex_home,
|
||||
r#"[features.network_proxy.credentials.vendor]
|
||||
env = ["VENDOR_PASSWORD"]
|
||||
patterns = ["^pin_[a-z]{8}$"]
|
||||
url_prefixes = ["https://fixed.example"]
|
||||
"#,
|
||||
)?;
|
||||
let mut app_server = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.with_args(&[
|
||||
"-c",
|
||||
"features.network_proxy.credentials.vendor.url_prefixes=['https://fixed.example']",
|
||||
])
|
||||
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
|
||||
.await?;
|
||||
let id = app_server
|
||||
.send_config_value_write_request(ConfigValueWriteParams {
|
||||
file_path: None,
|
||||
key_path: "features.network_proxy.credentials.vendor.url_prefixes".to_string(),
|
||||
value: json!(null),
|
||||
merge_strategy: MergeStrategy::Replace,
|
||||
expected_version: None,
|
||||
})
|
||||
.await?;
|
||||
let write: ConfigWriteResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(id)).await??;
|
||||
assert_eq!(write.status, WriteStatus::OkOverridden);
|
||||
|
||||
let saved: toml::Value = toml::from_str(&std::fs::read_to_string(
|
||||
codex_home.path().join("config.toml"),
|
||||
)?)?;
|
||||
assert_eq!(
|
||||
serde_json::to_value(&saved["features"]["network_proxy"]["credentials"]["vendor"])?,
|
||||
json!({ "env": ["VENDOR_PASSWORD"], "patterns": ["^pin_[a-z]{8}$"] }),
|
||||
);
|
||||
let id = app_server
|
||||
.send_config_read_request(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let read: ConfigReadResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(id)).await??;
|
||||
assert_eq!(
|
||||
read.config.additional["features"]["network_proxy"]["credentials"]["vendor"],
|
||||
json!({
|
||||
"env": ["VENDOR_PASSWORD"], "patterns": ["^pin_[a-z]{8}$"],
|
||||
"url_prefixes": ["https://fixed.example"], "auth": [],
|
||||
}),
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_provider_remapping_rejects_invalid_replacements_atomically() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let initial = r#"[features.network_proxy]
|
||||
enabled = true
|
||||
credential_broker = true
|
||||
[features.network_proxy.credentials.a_working]
|
||||
env = ["VENDOR_PASSWORD"]
|
||||
patterns = ["^pin_[a-z]{8}$"]
|
||||
url_prefixes = ["https://api.vendor.example"]
|
||||
"#;
|
||||
write_config(&codex_home, initial)?;
|
||||
let mut app_server = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_auto_env()
|
||||
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
|
||||
.await?;
|
||||
for (key_path, merge_strategy, replacement) in [
|
||||
json!({ "env": ["VENDOR_PASSWORD"] }),
|
||||
json!({ "env": ["VENDOR_PASSWORD"], "patterns": ["["], "url_prefixes": ["https://api.vendor.example"] }),
|
||||
json!({ "env": ["VENDOR_PASSWORD"], "patterns": ["^pin_[a-z]{8}$"], "url_prefixes": ["http://api.vendor.example"] }),
|
||||
json!({ "env": ["VENDOR_PASSWORD"], "patterns": ["^pin_[a-z]{8}$"], "url_prefixes": ["https://api.vendor.example"], "auth": ["header"] }),
|
||||
]
|
||||
.into_iter()
|
||||
.flat_map(|replacement| [
|
||||
("features.network_proxy.credentials.z_new", MergeStrategy::Upsert, replacement.clone()),
|
||||
("features.network_proxy.credentials.a_working", MergeStrategy::Replace, replacement.clone()),
|
||||
("features.network_proxy.credentials", MergeStrategy::Replace, json!({ "a_working": replacement })),
|
||||
])
|
||||
.chain([
|
||||
("features.network_proxy.credentials.a_working.patterns", MergeStrategy::Upsert, json!(["["])),
|
||||
("features.network_proxy.credentials.a_working.patterns", MergeStrategy::Replace, json!(["["])),
|
||||
("features.network_proxy.credentials.a_working.patterns", MergeStrategy::Replace, json!(null)),
|
||||
("features.network_proxy.credentials.a_working.url_prefixes", MergeStrategy::Upsert, json!(["http://api.vendor.example"])),
|
||||
]) {
|
||||
let id = app_server
|
||||
.send_config_value_write_request(ConfigValueWriteParams {
|
||||
file_path: None,
|
||||
key_path: key_path.to_string(),
|
||||
value: replacement,
|
||||
merge_strategy,
|
||||
expected_version: None,
|
||||
})
|
||||
.await?;
|
||||
let error: JSONRPCError = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
app_server.read_stream_until_error_message(RequestId::Integer(id)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(
|
||||
error
|
||||
.error
|
||||
.data
|
||||
.as_ref()
|
||||
.and_then(|data| data.get("config_write_error_code")),
|
||||
Some(&json!("configValidationError"))
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(codex_home.path().join("config.toml"))?,
|
||||
initial
|
||||
);
|
||||
}
|
||||
let id = app_server
|
||||
.send_config_batch_write_request(ConfigBatchWriteParams {
|
||||
file_path: None,
|
||||
edits: [
|
||||
("a_working", "OTHER_PASSWORD"),
|
||||
("b_incomplete", "VENDOR_PASSWORD"),
|
||||
]
|
||||
.into_iter()
|
||||
.map(|(provider, source)| ConfigEdit {
|
||||
key_path: format!("features.network_proxy.credentials.{provider}.env"),
|
||||
value: json!([source]),
|
||||
merge_strategy: MergeStrategy::Upsert,
|
||||
})
|
||||
.collect(),
|
||||
expected_version: None,
|
||||
reload_user_config: false,
|
||||
})
|
||||
.await?;
|
||||
let error: JSONRPCError = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
app_server.read_stream_until_error_message(RequestId::Integer(id)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(
|
||||
error
|
||||
.error
|
||||
.data
|
||||
.as_ref()
|
||||
.and_then(|data| data.get("config_write_error_code")),
|
||||
Some(&json!("configValidationError"))
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(codex_home.path().join("config.toml"))?,
|
||||
initial
|
||||
);
|
||||
let id = app_server
|
||||
.send_config_batch_write_request(ConfigBatchWriteParams {
|
||||
file_path: None,
|
||||
edits: [
|
||||
("env", json!(["VENDOR_PASSWORD"])),
|
||||
("patterns", json!(["^pin_[a-z]{8}$"])),
|
||||
("url_prefix_from_env", json!("VENDOR_ENDPOINT")),
|
||||
]
|
||||
.into_iter()
|
||||
.map(|(field, value)| ConfigEdit {
|
||||
key_path: format!("features.network_proxy.credentials.z_new.{field}"),
|
||||
value,
|
||||
merge_strategy: MergeStrategy::Upsert,
|
||||
})
|
||||
.collect(),
|
||||
expected_version: None,
|
||||
reload_user_config: false,
|
||||
})
|
||||
.await?;
|
||||
let written: ConfigWriteResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(id)).await??;
|
||||
assert_eq!(written.status, WriteStatus::Ok);
|
||||
let id = app_server
|
||||
.send_config_read_request(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let read: ConfigReadResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(id)).await??;
|
||||
assert_eq!(
|
||||
read.config.additional["features"]["network_proxy"]["credentials"],
|
||||
json!({
|
||||
"z_new": { "env": ["VENDOR_PASSWORD"], "patterns": ["^pin_[a-z]{8}$"], "url_prefix_from_env": "VENDOR_ENDPOINT", "url_prefixes": [], "auth": [] },
|
||||
})
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_provider_upsert_persists_displaced_siblings() -> Result<()> {
|
||||
let upsert = |path: &str, value| ConfigEdit {
|
||||
key_path: format!("features.network_proxy.credentials{path}"),
|
||||
value,
|
||||
merge_strategy: MergeStrategy::Upsert,
|
||||
};
|
||||
for (mut edits, remapped_source) in [
|
||||
(
|
||||
vec![upsert(".z_new", json!({ "env": ["VENDOR_PASSWORD"] }))],
|
||||
None,
|
||||
),
|
||||
(vec![upsert(".z_new.env", json!(["VENDOR_PASSWORD"]))], None),
|
||||
(
|
||||
vec![upsert(
|
||||
"",
|
||||
json!({ "z_new": { "env": ["VENDOR_PASSWORD"] } }),
|
||||
)],
|
||||
None,
|
||||
),
|
||||
(
|
||||
vec![upsert(
|
||||
"",
|
||||
json!({
|
||||
"a_old": { "env": ["NEW_PASSWORD"] },
|
||||
"z_new": { "env": ["VENDOR_PASSWORD"] },
|
||||
}),
|
||||
)],
|
||||
Some("NEW_PASSWORD"),
|
||||
),
|
||||
(
|
||||
vec![
|
||||
upsert(".z_new.env", json!(["VENDOR_PASSWORD"])),
|
||||
upsert(".a_old.env", json!(["OTHER_PASSWORD"])),
|
||||
],
|
||||
Some("OTHER_PASSWORD"),
|
||||
),
|
||||
(
|
||||
vec![
|
||||
upsert(".z_new", json!({ "env": ["VENDOR_PASSWORD"] })),
|
||||
ConfigEdit {
|
||||
key_path: "model".to_string(),
|
||||
value: json!("gpt-test"),
|
||||
merge_strategy: MergeStrategy::Replace,
|
||||
},
|
||||
upsert("", json!({ "a_old": { "env": ["OTHER_PASSWORD"] } })),
|
||||
],
|
||||
Some("OTHER_PASSWORD"),
|
||||
),
|
||||
] {
|
||||
let codex_home = TempDir::new()?;
|
||||
write_config(
|
||||
&codex_home,
|
||||
r#"[features.network_proxy]
|
||||
enabled = true
|
||||
credential_broker = true
|
||||
|
||||
[features.network_proxy.credentials.a_old]
|
||||
env = ["VENDOR_PASSWORD"]
|
||||
patterns = ["^old_[a-z]{8}$"]
|
||||
url_prefixes = ["https://old.example"]
|
||||
auth = ["token"]
|
||||
|
||||
[features.network_proxy.credentials.z_new]
|
||||
env = ["OTHER_PASSWORD"]
|
||||
patterns = ["^new_[a-z]{8}$"]
|
||||
url_prefixes = ["https://new.example"]
|
||||
auth = ["bearer"]
|
||||
"#,
|
||||
)?;
|
||||
let mut app_server = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_auto_env()
|
||||
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
|
||||
.await?;
|
||||
|
||||
let mut expected = json!({
|
||||
"z_new": {
|
||||
"auth": ["bearer"],
|
||||
"env": ["VENDOR_PASSWORD"],
|
||||
"patterns": ["^new_[a-z]{8}$"],
|
||||
"url_prefixes": ["https://new.example"],
|
||||
}
|
||||
});
|
||||
if let Some(source) = remapped_source {
|
||||
expected["a_old"] = json!({
|
||||
"auth": ["token"],
|
||||
"env": [source],
|
||||
"patterns": ["^old_[a-z]{8}$"],
|
||||
"url_prefixes": ["https://old.example"],
|
||||
});
|
||||
}
|
||||
|
||||
let write_id = if edits.len() == 1 {
|
||||
let edit = edits.pop().unwrap();
|
||||
app_server
|
||||
.send_config_value_write_request(ConfigValueWriteParams {
|
||||
file_path: None,
|
||||
key_path: edit.key_path,
|
||||
value: edit.value,
|
||||
merge_strategy: edit.merge_strategy,
|
||||
expected_version: None,
|
||||
})
|
||||
.await?
|
||||
} else {
|
||||
app_server
|
||||
.send_config_batch_write_request(ConfigBatchWriteParams {
|
||||
file_path: None,
|
||||
edits,
|
||||
expected_version: None,
|
||||
reload_user_config: false,
|
||||
})
|
||||
.await?
|
||||
};
|
||||
let write: ConfigWriteResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(write_id)).await??;
|
||||
assert_eq!(write.status, WriteStatus::Ok);
|
||||
|
||||
let read_id = app_server
|
||||
.send_config_read_request(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let read: ConfigReadResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(read_id)).await??;
|
||||
assert_eq!(
|
||||
read.config.additional["features"]["network_proxy"]["credentials"],
|
||||
expected
|
||||
);
|
||||
assert_eq!(
|
||||
read.origins["features.network_proxy.credentials.z_new.env.0"].version,
|
||||
write.version
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_value_write_updates_desktop_settings() -> Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
|
||||
Reference in new issue
Block a user