Skip to content

Commit 989f55d

Browse files
winston-openaiviyatb-oaicodex
authored
feat(network-proxy): experimental local credential broker (#28034)
## Why Codex child processes can inherit injectable local credentials directly, which lets commands read and exfiltrate the real values. This experimental slice keeps supported workflows working while moving those credentials behind the managed network proxy. This PR contains only the proxy-owned broker implementation. The Codex config and runtime integration is stacked separately in #29752. ## What changed - discover supported credentials during child setup, retain real values only in the in-memory proxy broker, and replace them with shaped dummy values - require a presented dummy to select a stored credential and preserve unrelated explicit authorization headers - bind GitHub cloud, GitHub Enterprise, and OpenAI credentials to their intended hosts - inject credentials only into TLS traffic by default; plaintext injection requires the explicit dangerous opt-in - use TLS ClientHello routing for CONNECT so non-TLS protocols remain opaque tunnels - expose a pure API that identifies environment keys still holding broker-generated dummies without mutating the caller's environment ## Scope - supported credentials: `GH_TOKEN`, `GITHUB_TOKEN`, `GH_ENTERPRISE_TOKEN`, `GITHUB_ENTERPRISE_TOKEN`, and `OPENAI_API_KEY` - GitHub cloud credentials match `github.com`, `api.github.com`, and `*.ghe.com` - GitHub Enterprise credentials match only the normalized non-cloud `GH_HOST` - OpenAI API keys match only `api.openai.com` - this does not cover SSH agents, kube client certificates, filesystem secret discovery, or context-injected secret scrubbing ## Validation - `just test -p codex-network-proxy` (191 passed) - focused opaque CONNECT, plaintext opt-in, dummy-selection, and child-isolation regressions passed - scoped Clippy check for `codex-network-proxy` passed --------- Co-authored-by: viyatb-oai <viyatb@openai.com> Co-authored-by: Codex <noreply@openai.com>
1 parent 8057603 commit 989f55d

16 files changed

Lines changed: 1378 additions & 112 deletions

File tree

codex-rs/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codex-rs/network-proxy/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ codex-utils-absolute-path = { workspace = true }
2121
codex-utils-home-dir = { workspace = true }
2222
codex-utils-rustls-provider = { workspace = true }
2323
globset = { workspace = true }
24+
rand = { workspace = true }
2425
serde = { workspace = true, features = ["derive"] }
2526
serde_json = { workspace = true }
2627
thiserror = { workspace = true }

codex-rs/network-proxy/src/config.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ pub struct NetworkProxyConfig {
2121
pub network: NetworkProxySettings,
2222
}
2323

24+
impl NetworkProxyConfig {
25+
pub fn set_credential_broker_enabled(&mut self, enabled: bool) {
26+
self.network.credential_broker = enabled;
27+
self.network.mitm |= enabled;
28+
}
29+
}
30+
2431
/// Variant order encodes effective precedence for duplicate patterns:
2532
/// `None < Allow < Deny`, so deny wins over allow when entries conflict.
2633
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
@@ -142,6 +149,10 @@ pub struct NetworkProxySettings {
142149
#[serde(default)]
143150
pub mitm: bool,
144151
#[serde(default)]
152+
pub credential_broker: bool,
153+
#[serde(default)]
154+
pub dangerously_allow_plaintext_credential_injection: bool,
155+
#[serde(default)]
145156
pub mitm_hooks: Vec<MitmHookConfig>,
146157
}
147158

@@ -161,6 +172,8 @@ impl Default for NetworkProxySettings {
161172
unix_sockets: None,
162173
allow_local_binding: false,
163174
mitm: false,
175+
credential_broker: false,
176+
dangerously_allow_plaintext_credential_injection: false,
164177
mitm_hooks: Vec::new(),
165178
}
166179
}
@@ -593,6 +606,8 @@ mod tests {
593606
unix_sockets: None,
594607
allow_local_binding: false,
595608
mitm: false,
609+
credential_broker: false,
610+
dangerously_allow_plaintext_credential_injection: false,
596611
mitm_hooks: Vec::new(),
597612
}
598613
);
@@ -658,6 +673,8 @@ mod tests {
658673
"unix_sockets": null,
659674
"allow_local_binding": false,
660675
"mitm": false,
676+
"credential_broker": false,
677+
"dangerously_allow_plaintext_credential_injection": false,
661678
"mitm_hooks": [],
662679
}
663680
})
Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
mod providers;
2+
3+
use crate::policy::normalize_host;
4+
use rama_http::HeaderMap;
5+
use std::collections::HashMap;
6+
use std::sync::Arc;
7+
use std::sync::RwLock;
8+
9+
pub const CREDENTIAL_BROKER_ACTIVE_ENV_KEY: &str = "CODEX_NETWORK_PROXY_CREDENTIAL_BROKER_ACTIVE";
10+
pub(crate) const BROKERED_CREDENTIALS_ENV_KEY: &str = "CODEX_NETWORK_PROXY_BROKERED_CREDENTIALS";
11+
12+
#[derive(Clone)]
13+
pub(crate) struct CredentialBroker {
14+
state: Arc<RwLock<CredentialBrokerState>>,
15+
}
16+
17+
#[derive(Default)]
18+
struct CredentialBrokerState {
19+
enabled: bool,
20+
credentials: Vec<CredentialRecord>,
21+
}
22+
23+
struct CredentialRecord {
24+
env_var: String,
25+
provider: &'static providers::CredentialProvider,
26+
host_binding: providers::CredentialHostBinding,
27+
real_value: String,
28+
dummy_value: String,
29+
}
30+
31+
impl CredentialBroker {
32+
pub(crate) fn new(enabled: bool) -> Self {
33+
Self {
34+
state: Arc::new(RwLock::new(CredentialBrokerState {
35+
enabled,
36+
..CredentialBrokerState::default()
37+
})),
38+
}
39+
}
40+
41+
pub(crate) fn enabled(&self) -> bool {
42+
self.read_state().enabled
43+
}
44+
45+
pub(crate) fn virtualize_child_env(&self, env: &mut HashMap<String, String>) {
46+
let mut state = self.write_state();
47+
if !state.enabled {
48+
env.remove(CREDENTIAL_BROKER_ACTIVE_ENV_KEY);
49+
env.remove(BROKERED_CREDENTIALS_ENV_KEY);
50+
return;
51+
}
52+
env.insert(
53+
CREDENTIAL_BROKER_ACTIVE_ENV_KEY.to_string(),
54+
"1".to_string(),
55+
);
56+
57+
for provider in providers::credential_providers() {
58+
for source in provider.sources() {
59+
if let Some(host_binding) = (source.host_binding)(env) {
60+
for env_var in source.env_vars {
61+
virtualize_env_var(
62+
env,
63+
&mut state,
64+
env_var,
65+
provider,
66+
host_binding.clone(),
67+
);
68+
}
69+
}
70+
}
71+
}
72+
update_brokered_credentials_marker(&state, env);
73+
}
74+
75+
pub(crate) fn host_requires_mitm(&self, host: &str) -> bool {
76+
let normalized_host = normalize_host(host);
77+
let state = self.read_state();
78+
state.enabled
79+
&& state
80+
.credentials
81+
.iter()
82+
.any(|credential| credential.matches_host(&normalized_host))
83+
}
84+
85+
pub(crate) fn inject_request_headers(&self, host: &str, headers: &mut HeaderMap) {
86+
let normalized_host = normalize_host(host);
87+
let state = self.read_state();
88+
if !state.enabled {
89+
return;
90+
}
91+
92+
let matching_credentials = state
93+
.credentials
94+
.iter()
95+
.filter(|credential| credential.matches_host(&normalized_host))
96+
.collect::<Vec<_>>();
97+
let Some(credential) = select_credential(headers, &matching_credentials) else {
98+
return;
99+
};
100+
let Some(header_value) = credential
101+
.provider
102+
.request_header_value(&credential.real_value)
103+
else {
104+
return;
105+
};
106+
credential
107+
.provider
108+
.insert_request_header(headers, header_value);
109+
}
110+
111+
fn read_state(&self) -> std::sync::RwLockReadGuard<'_, CredentialBrokerState> {
112+
self.state
113+
.read()
114+
.unwrap_or_else(std::sync::PoisonError::into_inner)
115+
}
116+
117+
fn write_state(&self) -> std::sync::RwLockWriteGuard<'_, CredentialBrokerState> {
118+
self.state
119+
.write()
120+
.unwrap_or_else(std::sync::PoisonError::into_inner)
121+
}
122+
}
123+
124+
fn virtualize_env_var(
125+
env: &mut HashMap<String, String>,
126+
state: &mut CredentialBrokerState,
127+
env_var: &str,
128+
provider: &'static providers::CredentialProvider,
129+
host_binding: providers::CredentialHostBinding,
130+
) {
131+
let Some(real_value) = brokerable_credential_value(env, state, env_var, provider) else {
132+
return;
133+
};
134+
135+
let dummy_value = state.register(env_var, provider, host_binding, real_value);
136+
env.insert(env_var.to_string(), dummy_value);
137+
}
138+
139+
fn brokerable_credential_value<'a>(
140+
env: &'a HashMap<String, String>,
141+
state: &CredentialBrokerState,
142+
env_var: &str,
143+
provider: &providers::CredentialProvider,
144+
) -> Option<&'a str> {
145+
let real_value = env.get(env_var)?.trim();
146+
(!real_value.is_empty()
147+
&& !state.is_dummy_value(real_value)
148+
&& provider.request_header_value(real_value).is_some())
149+
.then_some(real_value)
150+
}
151+
152+
impl CredentialBrokerState {
153+
fn register(
154+
&mut self,
155+
env_var: &str,
156+
provider: &'static providers::CredentialProvider,
157+
host_binding: providers::CredentialHostBinding,
158+
real_value: &str,
159+
) -> String {
160+
if let Some(existing) = self.credentials.iter().find(|credential| {
161+
credential.env_var == env_var
162+
&& std::ptr::eq(credential.provider, provider)
163+
&& credential.host_binding == host_binding
164+
&& credential.real_value == real_value
165+
}) {
166+
return existing.dummy_value.clone();
167+
}
168+
169+
let dummy_value = loop {
170+
let candidate = provider.dummy_value(real_value);
171+
if candidate != real_value && !self.is_dummy_value(&candidate) {
172+
break candidate;
173+
}
174+
};
175+
self.credentials.push(CredentialRecord {
176+
env_var: env_var.to_string(),
177+
provider,
178+
host_binding,
179+
real_value: real_value.to_string(),
180+
dummy_value: dummy_value.clone(),
181+
});
182+
dummy_value
183+
}
184+
185+
fn is_dummy_value(&self, value: &str) -> bool {
186+
self.credentials
187+
.iter()
188+
.any(|credential| credential.dummy_value == value)
189+
}
190+
}
191+
192+
impl CredentialRecord {
193+
fn matches_host(&self, host: &str) -> bool {
194+
self.host_binding.matches_host(host)
195+
}
196+
}
197+
198+
fn select_credential<'a>(
199+
headers: &HeaderMap,
200+
matching_credentials: &[&'a CredentialRecord],
201+
) -> Option<&'a CredentialRecord> {
202+
let dummy_matches = matching_credentials
203+
.iter()
204+
.copied()
205+
.filter(|credential| {
206+
credential
207+
.provider
208+
.request_header(headers)
209+
.and_then(|value| value.to_str().ok())
210+
.is_some_and(|value| value.contains(&credential.dummy_value))
211+
})
212+
.collect::<Vec<_>>();
213+
match dummy_matches.as_slice() {
214+
[credential] => Some(*credential),
215+
[] | [_, _, ..] => None,
216+
}
217+
}
218+
219+
fn update_brokered_credentials_marker(
220+
state: &CredentialBrokerState,
221+
env: &mut HashMap<String, String>,
222+
) {
223+
let brokered = providers::credential_broker_env_keys()
224+
.filter_map(|key| {
225+
let value = env.get(key)?;
226+
state.is_dummy_value(value).then_some((key, value.as_str()))
227+
})
228+
.collect::<Vec<_>>();
229+
match serde_json::to_string(&brokered) {
230+
Ok(marker) => {
231+
env.insert(BROKERED_CREDENTIALS_ENV_KEY.to_string(), marker);
232+
}
233+
Err(_) => {
234+
env.remove(BROKERED_CREDENTIALS_ENV_KEY);
235+
}
236+
}
237+
}
238+
239+
/// Returns supported environment keys whose current values still match the child-scoped dummy
240+
/// values recorded by the credential broker.
241+
///
242+
/// The broker marker is treated as untrusted: malformed metadata, unsupported keys, and values
243+
/// replaced by the user are ignored. The environment is not mutated; callers own the decision to
244+
/// remove the returned keys.
245+
pub fn brokered_credential_dummy_env_keys(env: &HashMap<String, String>) -> Vec<String> {
246+
env.get(BROKERED_CREDENTIALS_ENV_KEY)
247+
.and_then(|marker| serde_json::from_str::<Vec<(String, String)>>(marker).ok())
248+
.unwrap_or_default()
249+
.into_iter()
250+
.filter_map(|(key, dummy_value)| {
251+
(providers::credential_broker_env_keys().any(|candidate| candidate == key.as_str())
252+
&& env.get(&key) == Some(&dummy_value))
253+
.then_some(key)
254+
})
255+
.collect()
256+
}
257+
258+
/// Returns supported credential keys only for an environment with an active broker.
259+
pub fn brokered_credential_env_keys(
260+
env: &HashMap<String, String>,
261+
) -> impl Iterator<Item = &'static str> {
262+
let active = env
263+
.get(CREDENTIAL_BROKER_ACTIVE_ENV_KEY)
264+
.is_some_and(|value| value == "1");
265+
providers::credential_broker_env_keys().filter(move |_| active)
266+
}
267+
268+
#[cfg(test)]
269+
#[path = "credential_broker_tests.rs"]
270+
mod tests;

0 commit comments

Comments
 (0)