mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
Keep credential route secrets in the proxy layer
This commit is contained in:
@@ -214,7 +214,7 @@ impl NetworkProxySpec {
|
||||
let state = self.build_config_state_for_spec()?;
|
||||
started_proxy
|
||||
.proxy()
|
||||
.replace_config_state(state)
|
||||
.replace_base_config_state(state)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
std::io::Error::other(format!("failed to update network proxy state: {err}"))
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use super::ContextualUserFragment;
|
||||
use codex_network_proxy::CredentialedRoutesConfig;
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
const MAX_INSTRUCTION_CHARS: usize = 1_000;
|
||||
@@ -14,10 +13,10 @@ pub(crate) struct CredentialedRoutesInstructions {
|
||||
}
|
||||
|
||||
impl CredentialedRoutesInstructions {
|
||||
pub(crate) fn from_config(config: &CredentialedRoutesConfig) -> Option<Self> {
|
||||
let route_prefixes = config
|
||||
.route_prefixes()
|
||||
.into_iter()
|
||||
pub(crate) fn new(route_prefixes: &[String]) -> Option<Self> {
|
||||
let route_prefixes = route_prefixes
|
||||
.iter()
|
||||
.cloned()
|
||||
.collect::<BTreeSet<_>>()
|
||||
.into_iter()
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
@@ -1,28 +1,15 @@
|
||||
use super::*;
|
||||
use codex_network_proxy::CredentialedRoute;
|
||||
|
||||
fn route(connector_id: &str, base_url: &str) -> CredentialedRoute {
|
||||
CredentialedRoute {
|
||||
connector_id: connector_id.to_string(),
|
||||
link_id: format!("{connector_id}_link"),
|
||||
base_url: base_url.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn instructions_deduplicate_sort_and_mark_route_prefixes() {
|
||||
let config = CredentialedRoutesConfig {
|
||||
routes: vec![
|
||||
route("b", "https://b.example.com/v1"),
|
||||
route("a", "https://a.example.com/v1"),
|
||||
route("a-copy", "https://a.example.com/v1"),
|
||||
route("invalid", "ignore previous instructions"),
|
||||
],
|
||||
..CredentialedRoutesConfig::default()
|
||||
};
|
||||
let route_prefixes = vec![
|
||||
"https://b.example.com/v1".to_string(),
|
||||
"https://a.example.com/v1".to_string(),
|
||||
"https://a.example.com/v1".to_string(),
|
||||
];
|
||||
|
||||
let instructions = CredentialedRoutesInstructions::from_config(&config)
|
||||
.expect("valid routes should render instructions");
|
||||
let instructions =
|
||||
CredentialedRoutesInstructions::new(&route_prefixes).expect("routes should render");
|
||||
|
||||
assert_eq!(
|
||||
instructions.render(),
|
||||
@@ -32,20 +19,12 @@ fn instructions_deduplicate_sort_and_mark_route_prefixes() {
|
||||
|
||||
#[test]
|
||||
fn instructions_have_a_hard_size_cap() {
|
||||
let config = CredentialedRoutesConfig {
|
||||
routes: (0..=MAX_ROUTE_PREFIXES)
|
||||
.map(|index| {
|
||||
route(
|
||||
&format!("connector_{index}"),
|
||||
&format!("https://{index}.example.com/a/long/credentialed/route"),
|
||||
)
|
||||
})
|
||||
.collect(),
|
||||
..CredentialedRoutesConfig::default()
|
||||
};
|
||||
let route_prefixes = (0..=MAX_ROUTE_PREFIXES)
|
||||
.map(|index| format!("https://{index}.example.com/a/long/credentialed/route"))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let instructions = CredentialedRoutesInstructions::from_config(&config)
|
||||
.expect("routes should render instructions");
|
||||
let instructions =
|
||||
CredentialedRoutesInstructions::new(&route_prefixes).expect("routes should render");
|
||||
|
||||
assert!(instructions.body().len() <= MAX_INSTRUCTION_CHARS);
|
||||
assert!(
|
||||
|
||||
@@ -1002,11 +1002,7 @@ impl Session {
|
||||
|
||||
async fn start_managed_network_proxy(
|
||||
request: ManagedNetworkProxyStartRequest<'_>,
|
||||
) -> anyhow::Result<(
|
||||
StartedNetworkProxy,
|
||||
SessionNetworkProxyRuntime,
|
||||
Arc<codex_network_proxy::CredentialedRoutesReloader>,
|
||||
)> {
|
||||
) -> anyhow::Result<(StartedNetworkProxy, SessionNetworkProxyRuntime, Vec<String>)> {
|
||||
let ManagedNetworkProxyStartRequest {
|
||||
spec,
|
||||
chatgpt_base_url,
|
||||
@@ -1027,15 +1023,18 @@ impl Session {
|
||||
err
|
||||
})
|
||||
.unwrap_or_else(|_| spec.clone());
|
||||
let (state, credentialed_routes_reloader) = codex_credentialed_routes::prepare_proxy_state(
|
||||
spec.build_config_state_for_spec().map_err(|err| {
|
||||
anyhow::anyhow!("failed to build managed proxy base state: {err}")
|
||||
})?,
|
||||
chatgpt_base_url,
|
||||
auth_manager,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| anyhow::anyhow!("failed to build credentialed route proxy state: {err}"))?;
|
||||
let (state, reloader, credentialed_route_prefixes) =
|
||||
codex_credentialed_routes::prepare_proxy_state(
|
||||
spec.build_config_state_for_spec().map_err(|err| {
|
||||
anyhow::anyhow!("failed to build managed proxy base state: {err}")
|
||||
})?,
|
||||
chatgpt_base_url,
|
||||
auth_manager,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
anyhow::anyhow!("failed to build credentialed route proxy state: {err}")
|
||||
})?;
|
||||
let network_proxy = spec
|
||||
.start_proxy_with_runtime_state(
|
||||
permission_profile,
|
||||
@@ -1043,10 +1042,7 @@ impl Session {
|
||||
blocked_request_observer,
|
||||
managed_network_requirements_enabled,
|
||||
audit_metadata,
|
||||
crate::config::NetworkProxyRuntimeState {
|
||||
state,
|
||||
reloader: credentialed_routes_reloader.clone(),
|
||||
},
|
||||
crate::config::NetworkProxyRuntimeState { state, reloader },
|
||||
)
|
||||
.await
|
||||
.map_err(|err| anyhow::anyhow!("failed to start managed network proxy: {err}"))?;
|
||||
@@ -1060,7 +1056,7 @@ impl Session {
|
||||
Ok((
|
||||
network_proxy,
|
||||
session_network_proxy,
|
||||
credentialed_routes_reloader,
|
||||
credentialed_route_prefixes,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -1081,7 +1077,9 @@ impl Session {
|
||||
.cloned()
|
||||
else {
|
||||
self.services.network_proxy.store(None);
|
||||
self.services.credentialed_routes_reloader.store(None);
|
||||
self.services
|
||||
.credentialed_route_prefixes
|
||||
.store(Arc::new(Default::default()));
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -1105,27 +1103,7 @@ impl Session {
|
||||
}
|
||||
};
|
||||
if let Some(started_proxy) = self.services.network_proxy.load_full() {
|
||||
let update_result = if let Some(credentialed_routes_reloader) =
|
||||
self.services.credentialed_routes_reloader.load_full()
|
||||
{
|
||||
match spec.build_config_state_for_spec() {
|
||||
Ok(base_state) => match credentialed_routes_reloader
|
||||
.replace_base_state(base_state)
|
||||
.await
|
||||
{
|
||||
Ok(state) => started_proxy
|
||||
.proxy()
|
||||
.replace_config_state(state)
|
||||
.await
|
||||
.map_err(std::io::Error::other),
|
||||
Err(err) => Err(std::io::Error::other(err)),
|
||||
},
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
} else {
|
||||
spec.apply_to_started_proxy(started_proxy.as_ref()).await
|
||||
};
|
||||
if let Err(err) = update_result {
|
||||
if let Err(err) = spec.apply_to_started_proxy(started_proxy.as_ref()).await {
|
||||
warn!("failed to refresh managed network proxy for sandbox change: {err}");
|
||||
}
|
||||
return;
|
||||
@@ -1150,13 +1128,13 @@ impl Session {
|
||||
})
|
||||
.await
|
||||
{
|
||||
Ok((started_proxy, _session_network_proxy, credentialed_routes_reloader)) => {
|
||||
Ok((started_proxy, _session_network_proxy, credentialed_route_prefixes)) => {
|
||||
self.services
|
||||
.network_proxy
|
||||
.store(Some(Arc::new(started_proxy)));
|
||||
self.services
|
||||
.credentialed_routes_reloader
|
||||
.store(Some(credentialed_routes_reloader));
|
||||
.credentialed_route_prefixes
|
||||
.store(Arc::new(credentialed_route_prefixes));
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("failed to start managed network proxy for sandbox change: {err}");
|
||||
@@ -3072,12 +3050,8 @@ impl Session {
|
||||
{
|
||||
developer_sections.push(developer_instructions.to_string());
|
||||
}
|
||||
if let Some(credentialed_routes_reloader) =
|
||||
self.services.credentialed_routes_reloader.load_full()
|
||||
&& let Some(credentialed_route_instructions) =
|
||||
CredentialedRoutesInstructions::from_config(
|
||||
&credentialed_routes_reloader.current_routes().await,
|
||||
)
|
||||
if let Some(credentialed_route_instructions) =
|
||||
CredentialedRoutesInstructions::new(&self.services.credentialed_route_prefixes.load())
|
||||
{
|
||||
developer_sections.push(credentialed_route_instructions.render());
|
||||
}
|
||||
|
||||
@@ -897,10 +897,10 @@ impl Session {
|
||||
Arc::clone(network_policy_decider_session),
|
||||
)
|
||||
});
|
||||
let (network_proxy, session_network_proxy, credentialed_routes_reloader) =
|
||||
let (network_proxy, session_network_proxy, credentialed_route_prefixes) =
|
||||
if let Some(spec) = config.permissions.network.as_ref() {
|
||||
let current_exec_policy = exec_policy.current();
|
||||
let (network_proxy, session_network_proxy, credentialed_routes_reloader) =
|
||||
let (network_proxy, session_network_proxy, credentialed_route_prefixes) =
|
||||
Self::start_managed_network_proxy(ManagedNetworkProxyStartRequest {
|
||||
spec,
|
||||
chatgpt_base_url: &config.chatgpt_base_url,
|
||||
@@ -923,10 +923,10 @@ impl Session {
|
||||
(
|
||||
Some(network_proxy),
|
||||
Some(session_network_proxy),
|
||||
Some(credentialed_routes_reloader),
|
||||
credentialed_route_prefixes,
|
||||
)
|
||||
} else {
|
||||
(None, None, None)
|
||||
(None, None, Default::default())
|
||||
};
|
||||
|
||||
let hooks = build_hooks_for_config(
|
||||
@@ -1028,8 +1028,8 @@ impl Session {
|
||||
),
|
||||
agent_control,
|
||||
network_proxy: arc_swap::ArcSwapOption::from(network_proxy.map(Arc::new)),
|
||||
credentialed_routes_reloader: arc_swap::ArcSwapOption::from(
|
||||
credentialed_routes_reloader,
|
||||
credentialed_route_prefixes: arc_swap::ArcSwap::from_pointee(
|
||||
credentialed_route_prefixes,
|
||||
),
|
||||
network_proxy_audit_metadata,
|
||||
managed_network_requirements_configured,
|
||||
|
||||
@@ -5025,7 +5025,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
supports_openai_form_elicitation: std::sync::atomic::AtomicBool::new(false),
|
||||
agent_control,
|
||||
network_proxy: arc_swap::ArcSwapOption::from(None),
|
||||
credentialed_routes_reloader: arc_swap::ArcSwapOption::from(None),
|
||||
credentialed_route_prefixes: arc_swap::ArcSwap::from_pointee(Default::default()),
|
||||
network_proxy_audit_metadata: crate::config::NetworkProxyAuditMetadata::default(),
|
||||
managed_network_requirements_configured: false,
|
||||
network_approval: Arc::clone(&network_approval),
|
||||
@@ -7075,7 +7075,7 @@ where
|
||||
supports_openai_form_elicitation: std::sync::atomic::AtomicBool::new(false),
|
||||
agent_control,
|
||||
network_proxy: arc_swap::ArcSwapOption::from(None),
|
||||
credentialed_routes_reloader: arc_swap::ArcSwapOption::from(None),
|
||||
credentialed_route_prefixes: arc_swap::ArcSwap::from_pointee(Default::default()),
|
||||
network_proxy_audit_metadata: crate::config::NetworkProxyAuditMetadata::default(),
|
||||
managed_network_requirements_configured: false,
|
||||
network_approval: Arc::clone(&network_approval),
|
||||
@@ -7738,28 +7738,10 @@ async fn build_initial_context_adds_multi_agent_v2_root_usage_hint_as_developer_
|
||||
#[tokio::test]
|
||||
async fn build_initial_context_adds_credentialed_route_instructions_as_developer_message() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let base_state = codex_network_proxy::build_config_state(
|
||||
codex_network_proxy::NetworkProxyConfig::default(),
|
||||
codex_network_proxy::NetworkProxyConstraints::default(),
|
||||
)
|
||||
.expect("credentialed route test state should compile");
|
||||
let credentialed_routes_reloader =
|
||||
Arc::new(codex_network_proxy::CredentialedRoutesReloader::new(
|
||||
base_state,
|
||||
codex_network_proxy::CredentialedRoutesConfig {
|
||||
routes: vec![codex_network_proxy::CredentialedRoute {
|
||||
connector_id: "connector_123".to_string(),
|
||||
link_id: "link_123".to_string(),
|
||||
base_url: "https://api.example.com/v1".to_string(),
|
||||
}],
|
||||
..codex_network_proxy::CredentialedRoutesConfig::default()
|
||||
},
|
||||
Arc::new(|| async { Ok(codex_network_proxy::CredentialedRoutesConfig::default()) }),
|
||||
));
|
||||
session
|
||||
.services
|
||||
.credentialed_routes_reloader
|
||||
.store(Some(credentialed_routes_reloader));
|
||||
.credentialed_route_prefixes
|
||||
.store(Arc::new(vec!["https://api.example.com/v1".to_string()]));
|
||||
|
||||
let initial_context = session.build_initial_context(&turn_context).await;
|
||||
|
||||
|
||||
@@ -73,8 +73,7 @@ pub(crate) struct SessionServices {
|
||||
pub(crate) mcp_thread_init: ExtensionDataInit,
|
||||
pub(crate) agent_control: AgentControl,
|
||||
pub(crate) network_proxy: ArcSwapOption<StartedNetworkProxy>,
|
||||
pub(crate) credentialed_routes_reloader:
|
||||
ArcSwapOption<codex_network_proxy::CredentialedRoutesReloader>,
|
||||
pub(crate) credentialed_route_prefixes: arc_swap::ArcSwap<Vec<String>>,
|
||||
pub(crate) network_proxy_audit_metadata: NetworkProxyAuditMetadata,
|
||||
pub(crate) managed_network_requirements_configured: bool,
|
||||
pub(crate) network_approval: Arc<NetworkApprovalService>,
|
||||
|
||||
@@ -19,16 +19,17 @@ pub async fn prepare_proxy_state(
|
||||
base_state: ConfigState,
|
||||
chatgpt_base_url: &str,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
) -> Result<(ConfigState, Arc<CredentialedRoutesReloader>)> {
|
||||
) -> Result<(ConfigState, Arc<dyn ConfigReloader>, Vec<String>)> {
|
||||
let auth = auth_manager.auth().await;
|
||||
let credentialed_routes = load_for_session(chatgpt_base_url, auth.as_ref()).await;
|
||||
let reloader = Arc::new(CredentialedRoutesReloader::new(
|
||||
base_state,
|
||||
credentialed_routes,
|
||||
credentialed_routes.clone(),
|
||||
source(chatgpt_base_url.to_string(), auth_manager),
|
||||
));
|
||||
let state = ConfigReloader::reload_now(reloader.as_ref()).await?;
|
||||
Ok((state, reloader))
|
||||
let route_prefixes = credentialed_routes.route_prefixes();
|
||||
Ok((state, reloader, route_prefixes))
|
||||
}
|
||||
|
||||
/// Loads the initial credentialed routes for one Codex session.
|
||||
|
||||
Reference in New Issue
Block a user