mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
Mark built-in dummy credentials and strengthen collision checks (#56297)
## What changed - Add `masked` after the provider prefix in built-in dummy credentials, followed by at least 22 random alphanumeric characters. Stop copying punctuation from the real credential. - Check built-in dummy candidates against observed credential owners and discoverable configured-provider aliases, and reject overlaps with existing dummy credentials. Allow configured patterns to match a built-in dummy's format when no credential identity collides. ## Testing Extend regression coverage for short and opaque credentials, fresh dummy values, known and late credential collisions, alias discovery, header injection, and text restoration. Verify that explicit authorization values remain unchanged even when they resemble masked credentials. GitOrigin-RevId: 5080176f3ecbcdbb18943eab30136e15a4a3985e
This commit is contained in:
@@ -42,7 +42,7 @@ async fn remote_credential_masking_request_history(
|
||||
(
|
||||
"OPENAI_API_KEY",
|
||||
"sk-proj-executor-only-0000000000000000000000000000000000",
|
||||
"sk-proj-",
|
||||
"sk-proj-masked",
|
||||
)
|
||||
};
|
||||
const CALL_ID: &str = "print-credential";
|
||||
|
||||
@@ -862,7 +862,7 @@ async fn shell_snapshot_v2_remote_managed_proxy_uses_prepared_execution_context(
|
||||
.split_once('|')
|
||||
.context("command should expose the child API key")?;
|
||||
if expect_masking {
|
||||
assert!(child_api_key.starts_with("sk-proj-"));
|
||||
assert!(child_api_key.starts_with("sk-proj-masked"));
|
||||
assert_ne!(child_api_key, API_KEY);
|
||||
assert!(!stdout.contains(API_KEY));
|
||||
} else {
|
||||
|
||||
@@ -1642,6 +1642,44 @@ impl CredentialBrokerState {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut alias_identities = Vec::new();
|
||||
if matches!(&provider, BrokeredCredentialProvider::Builtin(_))
|
||||
&& !self.configured_providers.is_empty()
|
||||
{
|
||||
let context_keys = self.environment(existing_env).provider_context_keys;
|
||||
for (key, value) in existing_env {
|
||||
if key.eq_ignore_ascii_case("PATH")
|
||||
|| key.to_ascii_uppercase().ends_with("_PATH")
|
||||
|| crate::is_managed_proxy_env_var(key, value)
|
||||
|| context_keys
|
||||
.iter()
|
||||
.any(|context| env_key_matches(key, context))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
let known = matching::known_credential_matches(self, value, existing_env);
|
||||
let uncovered = matching::mask_known_credentials(value, &known);
|
||||
for configured in &self.configured_providers {
|
||||
for matched in configured.find_discoverable_credentials(&uncovered) {
|
||||
let value = &uncovered[matched.range.clone()];
|
||||
if value.contains('\0')
|
||||
|| matching::is_operational_path_match(
|
||||
&uncovered,
|
||||
matched.range.start,
|
||||
matched.range.end,
|
||||
)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
alias_identities.push((
|
||||
BrokeredCredentialProvider::Configured(Arc::clone(configured)),
|
||||
value.to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let Some(dummy_value) = (0..64).find_map(|_| {
|
||||
let candidate = provider.dummy_value(real_value)?;
|
||||
(candidate != real_value
|
||||
@@ -1656,10 +1694,51 @@ impl CredentialBrokerState {
|
||||
|| credential.real_value == candidate
|
||||
|| credential.contains_embedded_value(&candidate, &credential.dummy_value)
|
||||
|| credential.contains_embedded_value(&candidate, &credential.real_value)
|
||||
|| is_builtin_shaped_credential(real_value)
|
||||
&& credential.dummy_value.contains(&candidate)
|
||||
|| provider.contains_embedded_value(&credential.dummy_value, &candidate)
|
||||
})
|
||||
&& !credential_provider_definitions(self).any(|other| {
|
||||
!other.same_provider(&provider)
|
||||
&& other.recognizes_strictly_embedded_dummy_collision_in(&candidate)
|
||||
&& match &provider {
|
||||
BrokeredCredentialProvider::Builtin(_) => {
|
||||
// Check observed identities even when their sources have not registered.
|
||||
!credential_provider_definitions(self).any(|owner_provider| {
|
||||
self.credential_owners.iter().any(|owner| {
|
||||
let owns_key = match &owner_provider {
|
||||
BrokeredCredentialProvider::Builtin(provider) => {
|
||||
provider.sources().iter().any(|source| {
|
||||
source.env_vars.iter().any(|key| {
|
||||
env_key_matches(key, &owner.env_var)
|
||||
})
|
||||
})
|
||||
}
|
||||
BrokeredCredentialProvider::Configured(provider) => provider
|
||||
.config
|
||||
.env
|
||||
.iter()
|
||||
.any(|key| env_key_matches(key, &owner.env_var)),
|
||||
};
|
||||
owns_key
|
||||
&& (candidate == owner.real_value
|
||||
|| is_builtin_shaped_credential(&owner.real_value)
|
||||
&& candidate.contains(&owner.real_value)
|
||||
|| owner_provider.contains_embedded_value(
|
||||
&candidate,
|
||||
&owner.real_value,
|
||||
))
|
||||
})
|
||||
})
|
||||
}
|
||||
BrokeredCredentialProvider::Configured(_) => {
|
||||
!credential_provider_definitions(self).any(|other| {
|
||||
!other.same_provider(&provider)
|
||||
&& other.recognizes_strictly_embedded_dummy_collision_in(&candidate)
|
||||
})
|
||||
}
|
||||
}
|
||||
&& !alias_identities.iter().any(|(alias_provider, value)| {
|
||||
candidate == *value
|
||||
|| is_builtin_shaped_credential(value) && candidate.contains(value)
|
||||
|| alias_provider.contains_embedded_value(&candidate, value)
|
||||
}))
|
||||
.then_some(candidate)
|
||||
}) else {
|
||||
|
||||
@@ -2109,6 +2109,101 @@ fn configured_provider_does_not_register_a_credential_inside_an_existing_dummy()
|
||||
assert_eq!(env["SECONDARY_TOKEN"], overlapping_real);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn builtin_dummy_rejects_known_and_late_credential_collisions() {
|
||||
for (custom_real, known_before_builtin) in [("mask", true), ("mask", false), ("pass", false)] {
|
||||
let broker = broker_for(CredentialProviderConfig {
|
||||
env: vec!["CUSTOM_TOKEN".to_string()],
|
||||
patterns: if known_before_builtin {
|
||||
vec!["^[a-z]{4}$".to_string(), "^ghp_[A-Za-z0-9]+$".to_string()]
|
||||
} else {
|
||||
vec!["^(?:pass|mask)$".to_string()]
|
||||
},
|
||||
url_prefix_from_env: Some("CUSTOM_HOST".to_string()),
|
||||
..CredentialProviderConfig::default()
|
||||
});
|
||||
if known_before_builtin {
|
||||
let mut unbound =
|
||||
HashMap::from([("CUSTOM_TOKEN".to_string(), custom_real.to_string())]);
|
||||
broker.virtualize_child_env(&mut unbound);
|
||||
assert!(broker.read_state().credentials.is_empty());
|
||||
}
|
||||
let github_real = "ghp_abcdefghijklmnopqrstuvwxyz1234567890";
|
||||
let mut builtin_env = HashMap::from([("GH_TOKEN".to_string(), github_real.to_string())]);
|
||||
broker.virtualize_child_env(&mut builtin_env);
|
||||
let github_dummy = &builtin_env["GH_TOKEN"];
|
||||
if known_before_builtin {
|
||||
assert_eq!(github_dummy, github_real);
|
||||
assert!(broker.read_state().credentials.is_empty());
|
||||
continue;
|
||||
}
|
||||
assert!(github_dummy.starts_with("ghp_masked"));
|
||||
|
||||
// The earlier dummy is not carried, so environment containment cannot reject the overlap.
|
||||
let mut late_env = HashMap::from([
|
||||
("CUSTOM_TOKEN".to_string(), custom_real.to_string()),
|
||||
(
|
||||
"CUSTOM_HOST".to_string(),
|
||||
"api.provider.example".to_string(),
|
||||
),
|
||||
]);
|
||||
broker.virtualize_child_env(&mut late_env);
|
||||
assert_eq!(late_env["CUSTOM_TOKEN"], custom_real);
|
||||
assert_eq!(broker.read_state().credentials.len(), 1);
|
||||
|
||||
let mut headers = HeaderMap::from_iter([(
|
||||
AUTHORIZATION,
|
||||
HeaderValue::from_str(&format!("Bearer {github_dummy}")).unwrap(),
|
||||
)]);
|
||||
broker.inject_request_headers("api.github.com", &mut headers);
|
||||
assert_eq!(headers[AUTHORIZATION], format!("Bearer {github_real}"));
|
||||
let mut restored = github_dummy.clone();
|
||||
assert!(broker.restore_text(&mut restored));
|
||||
assert_eq!(restored, github_real);
|
||||
|
||||
let mut headers =
|
||||
HeaderMap::from_iter([(AUTHORIZATION, HeaderValue::from_static("Bearer mask"))]);
|
||||
broker.inject_request_headers("https://api.provider.example/", &mut headers);
|
||||
assert_eq!(headers[AUTHORIZATION], "Bearer mask");
|
||||
}
|
||||
|
||||
let broker = broker_for(CredentialProviderConfig {
|
||||
env: vec!["CUSTOM_TOKEN".to_string()],
|
||||
patterns: vec!["mask[a-z]{2}".to_string()],
|
||||
url_prefixes: vec!["https://custom.example".to_string()],
|
||||
..CredentialProviderConfig::default()
|
||||
});
|
||||
let github_real = "ghp_abcdefghijklmnopqrstuvwxyz1234567890";
|
||||
let mut env = HashMap::from([
|
||||
("GH_TOKEN".to_string(), github_real.to_string()),
|
||||
("CUSTOM_ALIAS".to_string(), "masked".to_string()),
|
||||
]);
|
||||
broker.virtualize_child_env(&mut env);
|
||||
assert_eq!(env["GH_TOKEN"], github_real);
|
||||
assert!(!env.contains_key("CUSTOM_TOKEN"));
|
||||
assert!(
|
||||
broker
|
||||
.read_state()
|
||||
.credentials
|
||||
.iter()
|
||||
.all(|credential| { credential.env_var == "CUSTOM_TOKEN" })
|
||||
);
|
||||
let alias_dummy = &env["CUSTOM_ALIAS"];
|
||||
assert_ne!(alias_dummy, "masked");
|
||||
let mut headers = HeaderMap::from_iter([(
|
||||
AUTHORIZATION,
|
||||
HeaderValue::from_str(&format!("Bearer {alias_dummy}")).unwrap(),
|
||||
)]);
|
||||
let original = headers.clone();
|
||||
broker.inject_request_headers("https://unrelated.example/", &mut headers);
|
||||
assert_eq!(headers, original);
|
||||
broker.inject_request_headers("https://custom.example/", &mut headers);
|
||||
assert_eq!(headers[AUTHORIZATION], "Bearer masked");
|
||||
let mut restored = alias_dummy.clone();
|
||||
assert!(broker.restore_text(&mut restored));
|
||||
assert_eq!(restored, "masked");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn configured_provider_reload_preserves_credentials_and_rejects_overlapping_sources() {
|
||||
let token = "provider_abcdefghijklmnopqrstuvwx";
|
||||
|
||||
@@ -202,15 +202,13 @@ pub(super) fn translate_standard_request_header(
|
||||
}
|
||||
|
||||
fn shaped_dummy_value(real_value: &str, prefix: &str, minimum_len: usize) -> String {
|
||||
let target_len = real_value.len().max(minimum_len).max(prefix.len() + 16);
|
||||
let mut dummy = format!("{prefix}masked");
|
||||
// Keep over 128 bits of randomness after the visible marker.
|
||||
let target_len = real_value.len().max(minimum_len).max(dummy.len() + 22);
|
||||
dummy.reserve(target_len - dummy.len());
|
||||
let mut rng = rand::rng();
|
||||
let mut dummy = String::with_capacity(target_len);
|
||||
dummy.push_str(prefix);
|
||||
for index in prefix.len()..target_len {
|
||||
let character = match real_value.as_bytes().get(index).copied() {
|
||||
Some(template) if !template.is_ascii_alphanumeric() => template,
|
||||
_ => DUMMY_ALPHANUMERIC[rng.random_range(0..DUMMY_ALPHANUMERIC.len())],
|
||||
};
|
||||
for _ in dummy.len()..target_len {
|
||||
let character = DUMMY_ALPHANUMERIC[rng.random_range(0..DUMMY_ALPHANUMERIC.len())];
|
||||
dummy.push(char::from(character));
|
||||
}
|
||||
dummy
|
||||
|
||||
@@ -34,16 +34,13 @@ fn authorization(headers: &HeaderMap) -> Option<&str> {
|
||||
|
||||
fn assert_credential_shape(real_value: &str, dummy_value: &str, prefix: &str) {
|
||||
assert_ne!(dummy_value, real_value);
|
||||
assert_eq!(dummy_value.len(), real_value.len());
|
||||
assert_eq!(&dummy_value[..prefix.len()], prefix);
|
||||
let same_shape = real_value
|
||||
.bytes()
|
||||
.zip(dummy_value.bytes())
|
||||
.skip(prefix.len())
|
||||
.all(|(real, dummy)| {
|
||||
real.is_ascii_alphanumeric() && dummy.is_ascii_alphanumeric() || real == dummy
|
||||
});
|
||||
assert!(same_shape);
|
||||
assert!(dummy_value.len() >= real_value.len());
|
||||
let marker = format!("{prefix}masked");
|
||||
let suffix = dummy_value
|
||||
.strip_prefix(&marker)
|
||||
.expect("masked credential");
|
||||
assert!(suffix.len() >= 22);
|
||||
assert!(suffix.bytes().all(|byte| byte.is_ascii_alphanumeric()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -754,8 +751,12 @@ fn virtualize_child_env_keeps_adjacent_provider_credentials_separate() {
|
||||
let dummy_bundle = env["AUTH_BUNDLE"].clone();
|
||||
assert!(!dummy_bundle.contains(github_token));
|
||||
assert!(!dummy_bundle.contains(openai_token));
|
||||
let github_dummy = &dummy_bundle[..github_token.len()];
|
||||
let openai_dummy = &dummy_bundle[github_token.len() + 1..];
|
||||
let (github_dummy, openai_suffix) = dummy_bundle
|
||||
.split_once("_sk-proj-")
|
||||
.expect("separate provider credentials");
|
||||
let openai_dummy = format!("sk-proj-{openai_suffix}");
|
||||
assert_credential_shape(github_token, github_dummy, "ghp_");
|
||||
assert_credential_shape(openai_token, &openai_dummy, "sk-proj-");
|
||||
|
||||
let mut github_headers = headers_with_bearer(github_dummy);
|
||||
broker.inject_request_headers("api.github.com", &mut github_headers);
|
||||
@@ -764,7 +765,7 @@ fn virtualize_child_env_keeps_adjacent_provider_credentials_separate() {
|
||||
Some(format!("Bearer {github_token}").as_str())
|
||||
);
|
||||
|
||||
let mut openai_headers = headers_with_bearer(openai_dummy);
|
||||
let mut openai_headers = headers_with_bearer(&openai_dummy);
|
||||
broker.inject_request_headers("api.openai.com", &mut openai_headers);
|
||||
assert_eq!(
|
||||
authorization(&openai_headers),
|
||||
@@ -898,13 +899,42 @@ fn brokered_credential_value_env_keys_include_dummy_aliases() {
|
||||
|
||||
#[test]
|
||||
fn virtualize_child_env_uses_fresh_dummy_capabilities() {
|
||||
let mut first_env = env_map([("OPENAI_API_KEY", "sk-proj-abcdefghijklmnopqrstuvwxyz")]);
|
||||
let mut second_env = first_env.clone();
|
||||
for (source, real_value, prefix) in [
|
||||
("GH_TOKEN", "ghp_short", "ghp_"),
|
||||
("GH_TOKEN", "opaque:.credential/🦀", "ghp_"),
|
||||
("OPENAI_API_KEY", "sk-proj-short", "sk-proj-"),
|
||||
("OPENAI_API_KEY", "sk-svcacct-short", "sk-svcacct-"),
|
||||
("OPENAI_API_KEY", "opaque:.credential/🦀", "sk-"),
|
||||
] {
|
||||
let mut first_env = env_map([(source, real_value)]);
|
||||
let mut second_env = first_env.clone();
|
||||
|
||||
CredentialBroker::new(/*enabled*/ true).virtualize_child_env(&mut first_env);
|
||||
CredentialBroker::new(/*enabled*/ true).virtualize_child_env(&mut second_env);
|
||||
for env in [&mut first_env, &mut second_env] {
|
||||
let broker = CredentialBroker::new(/*enabled*/ true);
|
||||
broker.configure(&NetworkProxyConfig {
|
||||
credential_broker: true,
|
||||
credential_providers: BTreeMap::from([(
|
||||
"custom".to_string(),
|
||||
CredentialProviderConfig {
|
||||
env: vec!["CUSTOM_TOKEN".to_string()],
|
||||
patterns: vec![
|
||||
"^ghp_[A-Za-z0-9]+$".to_string(),
|
||||
"^sk-proj-[A-Za-z0-9]+$".to_string(),
|
||||
"^[a-z]{4}$".to_string(),
|
||||
],
|
||||
url_prefixes: vec!["https://custom.example".to_string()],
|
||||
..CredentialProviderConfig::default()
|
||||
},
|
||||
)]),
|
||||
..NetworkProxyConfig::default()
|
||||
});
|
||||
broker.virtualize_child_env(env);
|
||||
}
|
||||
|
||||
assert_ne!(first_env["OPENAI_API_KEY"], second_env["OPENAI_API_KEY"]);
|
||||
assert_credential_shape(real_value, &first_env[source], prefix);
|
||||
assert_credential_shape(real_value, &second_env[source], prefix);
|
||||
assert_ne!(first_env[source], second_env[source]);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1076,10 +1106,14 @@ fn inject_request_headers_requires_dummy_and_preserves_explicit_authorization()
|
||||
broker.inject_request_headers("api.openai.com", &mut headers);
|
||||
assert_eq!(authorization(&headers), Some("Bearer sk-real"));
|
||||
|
||||
let mut explicit_headers = headers_with_bearer("sk-explicit");
|
||||
broker.inject_request_headers("api.openai.com", &mut explicit_headers);
|
||||
|
||||
assert_eq!(authorization(&explicit_headers), Some("Bearer sk-explicit"));
|
||||
for explicit_value in [
|
||||
"sk-explicit",
|
||||
"sk-masked000000000000000000000000000000000000000000",
|
||||
] {
|
||||
let mut explicit_headers = headers_with_bearer(explicit_value);
|
||||
broker.inject_request_headers("api.openai.com", &mut explicit_headers);
|
||||
assert_eq!(explicit_headers, headers_with_bearer(explicit_value));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user