From 60c59733f2a2658ac5cecee6a1f5524b4e012132 Mon Sep 17 00:00:00 2001 From: Eva Wong Date: Fri, 1 May 2026 12:23:19 -0700 Subject: [PATCH] Preserve MITM hook order --- codex-rs/Cargo.lock | 1 + codex-rs/config/Cargo.toml | 3 +- codex-rs/config/src/permissions_toml.rs | 19 +++++---- codex-rs/core/src/config/config_tests.rs | 52 +++++++++++++++++++++-- codex-rs/core/src/network_proxy_loader.rs | 17 +++----- 5 files changed, 69 insertions(+), 23 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 50de8dc946..9eecd0eec8 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2432,6 +2432,7 @@ dependencies = [ "dunce", "futures", "gethostname", + "indexmap 2.13.0", "libc", "multimap", "pretty_assertions", diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index c345f31c6e..f77a76b764 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -28,6 +28,7 @@ codex-utils-path = { workspace = true } dunce = { workspace = true } futures = { workspace = true, features = ["alloc", "std"] } gethostname = { workspace = true } +indexmap = { workspace = true, features = ["serde"] } multimap = { workspace = true } prost = "0.14.3" schemars = { workspace = true } @@ -38,7 +39,7 @@ serde_path_to_error = { workspace = true } sha2 = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["fs"] } -toml = { workspace = true } +toml = { workspace = true, features = ["preserve_order"] } toml_edit = { workspace = true } tonic = { workspace = true } tonic-prost = { workspace = true } diff --git a/codex-rs/config/src/permissions_toml.rs b/codex-rs/config/src/permissions_toml.rs index 4a9070cbae..f73500b881 100644 --- a/codex-rs/config/src/permissions_toml.rs +++ b/codex-rs/config/src/permissions_toml.rs @@ -11,6 +11,7 @@ use codex_network_proxy::NetworkProxyConfig; use codex_network_proxy::NetworkUnixSocketPermission as ProxyNetworkUnixSocketPermission; use codex_network_proxy::normalize_host; use codex_protocol::permissions::FileSystemAccessMode; +use indexmap::IndexMap; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -184,15 +185,17 @@ pub struct NetworkToml { #[derive(Serialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct NetworkMitmToml { - pub hooks: Option>, - pub actions: Option>, + #[schemars(with = "Option>")] + pub hooks: Option>, + #[schemars(with = "Option>")] + pub actions: Option>, } #[derive(Deserialize)] #[serde(deny_unknown_fields)] struct NetworkMitmTomlUnchecked { - pub hooks: Option>, - pub actions: Option>, + pub hooks: Option>, + pub actions: Option>, } #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] @@ -282,7 +285,7 @@ impl NetworkMitmToml { pub fn validate_action_references( &self, - actions_by_name: &BTreeMap, + actions_by_name: &IndexMap, ) -> Result<(), String> { self.validate_action_definitions()?; @@ -305,7 +308,7 @@ impl NetworkMitmToml { pub fn to_runtime_hooks( &self, - actions_by_name: Option<&BTreeMap>, + actions_by_name: Option<&IndexMap>, ) -> Vec { self.hooks .as_ref() @@ -394,7 +397,7 @@ impl NetworkToml { impl NetworkMitmHookToml { fn to_runtime( &self, - actions_by_name: Option<&BTreeMap>, + actions_by_name: Option<&IndexMap>, ) -> MitmHookConfig { MitmHookConfig { host: self.host.clone(), @@ -411,7 +414,7 @@ impl NetworkMitmHookToml { fn selected_actions( &self, - actions_by_name: Option<&BTreeMap>, + actions_by_name: Option<&IndexMap>, ) -> MitmHookActionsConfig { let Some(actions_by_name) = actions_by_name else { return MitmHookActionsConfig::default(); diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 9f1db002d0..e7582be33b 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -98,6 +98,7 @@ use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::TempDirExt; use core_test_support::test_absolute_path; +use indexmap::IndexMap; use pretty_assertions::assert_eq; use rmcp::model::ElicitationCapability; use rmcp::model::FormElicitationCapability; @@ -819,7 +820,7 @@ strip_request_headers = ["authorization"] unix_sockets: None, allow_local_binding: None, mitm: Some(NetworkMitmToml { - hooks: Some(BTreeMap::from([( + hooks: Some(IndexMap::from([( "github_write".to_string(), NetworkMitmHookToml { host: "api.github.com".to_string(), @@ -831,7 +832,7 @@ strip_request_headers = ["authorization"] action: vec!["strip_auth".to_string()], }, )])), - actions: Some(BTreeMap::from([( + actions: Some(IndexMap::from([( "strip_auth".to_string(), NetworkMitmActionToml { strip_request_headers: vec!["authorization".to_string()], @@ -900,7 +901,7 @@ fn permissions_profile_network_to_proxy_config_preserves_mitm_hooks() { let network = NetworkToml { mode: Some(NetworkMode::Full), mitm: Some(NetworkMitmToml { - hooks: Some(BTreeMap::from([( + hooks: Some(IndexMap::from([( "github_write".to_string(), NetworkMitmHookToml { host: "api.github.com".to_string(), @@ -910,7 +911,7 @@ fn permissions_profile_network_to_proxy_config_preserves_mitm_hooks() { ..NetworkMitmHookToml::default() }, )])), - actions: Some(BTreeMap::from([( + actions: Some(IndexMap::from([( "strip_auth".to_string(), NetworkMitmActionToml { strip_request_headers: vec!["authorization".to_string()], @@ -937,6 +938,49 @@ fn permissions_profile_network_to_proxy_config_preserves_mitm_hooks() { ); } +#[test] +fn permissions_profile_network_to_proxy_config_preserves_mitm_hook_declaration_order() { + let toml = r#" +default_permissions = "workspace" + +[permissions.workspace.network.mitm.actions.noop] +strip_request_headers = ["authorization"] + +[permissions.workspace.network.mitm.hooks.z_first] +host = "api.github.com" +methods = ["POST"] +path_prefixes = ["/repos/openai/"] +action = ["noop"] + +[permissions.workspace.network.mitm.hooks.a_second] +host = "api.github.com" +methods = ["POST"] +path_prefixes = ["/repos/"] +action = ["noop"] +"#; + let cfg: ConfigToml = toml::from_str(toml).expect("permissions profile should deserialize"); + let permissions = cfg.permissions.expect("permissions should deserialize"); + let network = permissions + .entries + .get("workspace") + .expect("workspace profile should exist") + .network + .as_ref() + .expect("network profile should exist"); + + let config = network.to_network_proxy_config(); + + assert_eq!(config.network.mitm_hooks.len(), 2); + assert_eq!( + config.network.mitm_hooks[0].matcher.path_prefixes, + vec!["/repos/openai/".to_string()] + ); + assert_eq!( + config.network.mitm_hooks[1].matcher.path_prefixes, + vec!["/repos/".to_string()] + ); +} + #[tokio::test] async fn permissions_profiles_proxy_policy_does_not_start_managed_network_proxy_without_feature() -> std::io::Result<()> { diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 429becfef6..c40f56b4e9 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -31,8 +31,8 @@ use codex_network_proxy::build_config_state; use codex_network_proxy::normalize_host; use codex_network_proxy::validate_policy_against_constraints; use codex_utils_absolute_path::AbsolutePathBuf; +use indexmap::IndexMap; use serde::Deserialize; -use std::collections::BTreeMap; use std::sync::Arc; use tokio::sync::RwLock; @@ -214,8 +214,8 @@ fn apply_network_tables(config: &mut NetworkProxyConfig, parsed: NetworkTablesTo #[derive(Default)] struct NetworkConfigAccumulator { config: NetworkProxyConfig, - mitm_hooks: BTreeMap, - mitm_actions: BTreeMap, + mitm_hooks: IndexMap, + mitm_actions: IndexMap, } impl NetworkConfigAccumulator { @@ -242,17 +242,14 @@ impl NetworkConfigAccumulator { fn finish(mut self) -> Result { if !self.mitm_hooks.is_empty() { + let actions = self.mitm_actions; let mitm = NetworkMitmToml { hooks: Some(self.mitm_hooks), - actions: Some(self.mitm_actions), + actions: Some(actions.clone()), }; - let actions = mitm - .actions - .as_ref() - .expect("effective MITM actions should be present"); - mitm.validate_action_references(actions) + mitm.validate_action_references(&actions) .map_err(anyhow::Error::msg)?; - self.config.network.mitm_hooks = mitm.to_runtime_hooks(Some(actions)); + self.config.network.mitm_hooks = mitm.to_runtime_hooks(Some(&actions)); } self.config.network.mitm = self.config.network.mode == NetworkMode::Limited