mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Enforce environment MCP policies (#39335)
## What changed - Add environment-provided MCP restrictions for configured and plugin-provided servers. - Disable attachment-scoped servers while their environment configuration is pending, failed, or unselected, while preserving selected-plugin access and the controller-owned Apps server. - Apply the resolved policy to runtime startup, model tool exposure, telemetry, OAuth flows, and skill dependency installation. ## Testing - Cover policy filtering for configured and plugin servers, environment state transitions, Apps ownership, and managed OAuth rejection. GitOrigin-RevId: 7ca5d4dda14068e758497f88835d5cba685e159e
This commit is contained in:
@@ -49,6 +49,7 @@ use rmcp::transport::StreamableHttpService;
|
||||
use rmcp::transport::streamable_http_server::session::local::LocalSessionManager;
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
use test_case::test_case;
|
||||
use tokio::net::TcpListener;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::task::JoinHandle;
|
||||
@@ -151,16 +152,22 @@ async fn oauth_login_uses_http_headers_helper() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(false; "plain HTTP")]
|
||||
#[test_case(true; "HTTP headers helper")]
|
||||
#[tokio::test]
|
||||
async fn oauth_login_does_not_run_helper_disabled_by_managed_requirements() -> Result<()> {
|
||||
async fn oauth_login_rejects_servers_disabled_by_managed_requirements(
|
||||
with_headers_helper: bool,
|
||||
) -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let marker = codex_home.path().join("helper-ran");
|
||||
let helper = toml::Value::String(format!("echo invoked > \"{}\"", marker.display()));
|
||||
let helper = with_headers_helper
|
||||
.then(|| toml::Value::String(format!("echo invoked > \"{}\"", marker.display())))
|
||||
.map_or_else(String::new, |command| {
|
||||
format!("http_headers_helper = {command}\n")
|
||||
});
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
format!(
|
||||
"[mcp_servers.blocked]\nurl = \"https://example.com/mcp\"\nhttp_headers_helper = {helper}\n"
|
||||
),
|
||||
format!("[mcp_servers.blocked]\nurl = \"https://example.com/mcp\"\n{helper}"),
|
||||
)?;
|
||||
std::fs::write(
|
||||
codex_home.path().join("requirements.toml"),
|
||||
|
||||
@@ -4,6 +4,11 @@ use std::collections::BTreeSet;
|
||||
use std::collections::HashMap;
|
||||
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::McpServerDisabledReason;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_protocol::mcp_policy::EnvironmentMcpPolicy;
|
||||
|
||||
use crate::CODEX_APPS_MCP_SERVER_NAME;
|
||||
|
||||
/// Plugin identity retained with an MCP registration for tool attribution.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
@@ -56,6 +61,7 @@ pub enum McpServerSource {
|
||||
},
|
||||
Extension {
|
||||
id: String,
|
||||
host_owned_apps: bool,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -69,6 +75,19 @@ impl McpServerSource {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn is_host_owned_apps(&self, name: &str, config: &McpServerConfig) -> bool {
|
||||
name == CODEX_APPS_MCP_SERVER_NAME
|
||||
&& config.is_local_environment()
|
||||
&& matches!(
|
||||
self,
|
||||
Self::Compatibility { .. }
|
||||
| Self::Extension {
|
||||
host_owned_apps: true,
|
||||
..
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
fn disabled_registration_is_name_veto(&self) -> bool {
|
||||
// A selected package's policy applies to its registration, not to a higher runtime source
|
||||
// that happens to use the same logical server name.
|
||||
@@ -166,7 +185,28 @@ impl McpServerRegistration {
|
||||
) -> Self {
|
||||
Self::new(
|
||||
name,
|
||||
McpServerSource::Extension { id: id.into() },
|
||||
McpServerSource::Extension {
|
||||
id: id.into(),
|
||||
host_owned_apps: false,
|
||||
},
|
||||
config,
|
||||
RegistrationPrecedence::Extension(contribution_order),
|
||||
)
|
||||
}
|
||||
|
||||
/// Registers the controller-owned Apps server contributed by a host extension.
|
||||
pub fn from_hosted_apps(
|
||||
id: impl Into<String>,
|
||||
contribution_order: usize,
|
||||
config: McpServerConfig,
|
||||
) -> Self {
|
||||
let host_owned_apps = config.is_local_environment();
|
||||
Self::new(
|
||||
CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
McpServerSource::Extension {
|
||||
id: id.into(),
|
||||
host_owned_apps,
|
||||
},
|
||||
config,
|
||||
RegistrationPrecedence::Extension(contribution_order),
|
||||
)
|
||||
@@ -187,6 +227,19 @@ impl McpServerRegistration {
|
||||
}
|
||||
}
|
||||
|
||||
/// The authority available for MCP servers running in one environment.
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
pub enum McpEnvironmentAuthority<'a> {
|
||||
/// The selected environment adds no restrictions to the controller policy.
|
||||
Unrestricted,
|
||||
/// The owner supplied the final restrictions for this environment.
|
||||
Restricted(&'a EnvironmentMcpPolicy),
|
||||
/// An explicitly selected plugin can use its executor without attaching that executor.
|
||||
SelectedPluginsOnly,
|
||||
/// The attachment is pending or failed, so its owner policy is not available.
|
||||
Unavailable,
|
||||
}
|
||||
|
||||
/// One side of an MCP server conflict, including whether it registers or
|
||||
/// removes the server.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
@@ -272,11 +325,85 @@ impl McpCatalogBuilder {
|
||||
) {
|
||||
self.actions.push(CatalogAction::Remove {
|
||||
name,
|
||||
source: McpServerSource::Extension { id: id.into() },
|
||||
source: McpServerSource::Extension {
|
||||
id: id.into(),
|
||||
host_owned_apps: false,
|
||||
},
|
||||
precedence: RegistrationPrecedence::Extension(contribution_order),
|
||||
});
|
||||
}
|
||||
|
||||
/// Applies environment authority before resolving immutable server registrations.
|
||||
pub fn build_with_environment_authority<'a>(
|
||||
mut self,
|
||||
mut authority_for_environment: impl FnMut(&str) -> McpEnvironmentAuthority<'a>,
|
||||
) -> ResolvedMcpCatalog {
|
||||
for action in &mut self.actions {
|
||||
let CatalogAction::Register(registration) = action else {
|
||||
continue;
|
||||
};
|
||||
// Controller-owned Apps and existing managed denials are not attachment-owned.
|
||||
if !registration.config.enabled
|
||||
|| registration
|
||||
.source
|
||||
.is_host_owned_apps(®istration.name, ®istration.config)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
let allowed = match authority_for_environment(®istration.config.environment_id) {
|
||||
McpEnvironmentAuthority::Unrestricted => true,
|
||||
McpEnvironmentAuthority::SelectedPluginsOnly => {
|
||||
matches!(®istration.source, McpServerSource::SelectedPlugin(_))
|
||||
}
|
||||
McpEnvironmentAuthority::Unavailable => false,
|
||||
McpEnvironmentAuthority::Restricted(policy) => match ®istration.source {
|
||||
McpServerSource::Config
|
||||
| McpServerSource::Compatibility { .. }
|
||||
| McpServerSource::Extension { .. } => {
|
||||
policy.servers.as_ref().is_none_or(|requirements| {
|
||||
requirements
|
||||
.get(®istration.name)
|
||||
.is_some_and(|requirement| {
|
||||
registration.config.matches_requirement(requirement)
|
||||
})
|
||||
})
|
||||
}
|
||||
McpServerSource::Plugin(attribution)
|
||||
| McpServerSource::SelectedPlugin(attribution) => {
|
||||
// Empty server policy denies every plugin; otherwise use package policy.
|
||||
!policy.servers.as_ref().is_some_and(BTreeMap::is_empty)
|
||||
&& policy
|
||||
.plugins
|
||||
.as_ref()
|
||||
.filter(|plugins| {
|
||||
plugins.values().any(|plugin| plugin.mcp_servers.is_some())
|
||||
})
|
||||
.is_none_or(|plugins| {
|
||||
plugins
|
||||
.get(attribution.plugin_id())
|
||||
.and_then(|plugin| plugin.mcp_servers.as_ref())
|
||||
.and_then(|requirements| {
|
||||
requirements.get(®istration.name)
|
||||
})
|
||||
.is_some_and(|requirement| {
|
||||
registration.config.matches_requirement(requirement)
|
||||
})
|
||||
})
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
if !allowed {
|
||||
registration.config.enabled = false;
|
||||
registration.config.disabled_reason = Some(McpServerDisabledReason::Requirements {
|
||||
source: RequirementSource::Unknown,
|
||||
});
|
||||
}
|
||||
}
|
||||
self.build()
|
||||
}
|
||||
|
||||
pub fn build(mut self) -> ResolvedMcpCatalog {
|
||||
// Stable sorting makes action order the tie-breaker when precedence is equal.
|
||||
self.actions.sort_by_key(CatalogAction::precedence);
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -6,8 +7,13 @@ use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::McpServerToolConfig;
|
||||
use codex_config::McpServerTransportConfig;
|
||||
use codex_protocol::mcp_policy::EnvironmentMcpPolicy;
|
||||
use codex_protocol::mcp_policy::PluginMcpRequirements;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use crate::CODEX_APPS_MCP_SERVER_NAME;
|
||||
|
||||
use super::McpEnvironmentAuthority;
|
||||
use super::McpPluginAttribution;
|
||||
use super::McpServerConflict;
|
||||
use super::McpServerConflictAction;
|
||||
@@ -65,7 +71,10 @@ fn compatibility_source(id: &str) -> McpServerSource {
|
||||
}
|
||||
|
||||
fn extension_source(id: &str) -> McpServerSource {
|
||||
McpServerSource::Extension { id: id.to_string() }
|
||||
McpServerSource::Extension {
|
||||
id: id.to_string(),
|
||||
host_owned_apps: false,
|
||||
}
|
||||
}
|
||||
|
||||
fn register(source: McpServerSource) -> McpServerConflictAction {
|
||||
@@ -117,6 +126,7 @@ fn source_precedence_preserves_the_winning_registration() {
|
||||
resolved.source(),
|
||||
&McpServerSource::Extension {
|
||||
id: "hosted".to_string(),
|
||||
host_owned_apps: false,
|
||||
}
|
||||
);
|
||||
assert_eq!(resolved.config(), &extension);
|
||||
@@ -407,3 +417,79 @@ fn equal_precedence_uses_insertion_order_not_source_identity() {
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn environment_policy_exempts_only_explicitly_host_owned_apps() {
|
||||
let policy = EnvironmentMcpPolicy {
|
||||
servers: Some(BTreeMap::new()),
|
||||
plugins: None,
|
||||
};
|
||||
for (registration, expected) in [
|
||||
(
|
||||
McpServerRegistration::from_extension(
|
||||
CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
"apps",
|
||||
/*contribution_order*/ 0,
|
||||
server("https://apps.example/mcp"),
|
||||
),
|
||||
false,
|
||||
),
|
||||
(
|
||||
McpServerRegistration::from_hosted_apps(
|
||||
"apps",
|
||||
/*contribution_order*/ 0,
|
||||
server("https://apps.example/mcp"),
|
||||
),
|
||||
true,
|
||||
),
|
||||
] {
|
||||
let mut builder = ResolvedMcpCatalog::builder();
|
||||
builder.register(registration);
|
||||
let catalog = builder
|
||||
.build_with_environment_authority(|_| McpEnvironmentAuthority::Restricted(&policy));
|
||||
assert_eq!(
|
||||
catalog
|
||||
.server(CODEX_APPS_MCP_SERVER_NAME)
|
||||
.expect("Apps registration")
|
||||
.config()
|
||||
.enabled,
|
||||
expected
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn environment_policy_preserves_selected_plugin_and_empty_server_allowlist_semantics() {
|
||||
let mut builder = ResolvedMcpCatalog::builder();
|
||||
builder.register(McpServerRegistration::from_selected_plugin(
|
||||
"selected".to_string(),
|
||||
plugin("selected-plugin"),
|
||||
/*selection_order*/ 0,
|
||||
server("https://plugin.example/mcp"),
|
||||
));
|
||||
let metadata_only_policy = EnvironmentMcpPolicy {
|
||||
servers: None,
|
||||
plugins: Some(BTreeMap::from([(
|
||||
"metadata-only-plugin".to_string(),
|
||||
PluginMcpRequirements { mcp_servers: None },
|
||||
)])),
|
||||
};
|
||||
let deny_all_policy = EnvironmentMcpPolicy {
|
||||
servers: Some(BTreeMap::new()),
|
||||
plugins: None,
|
||||
};
|
||||
|
||||
for (policy, expected) in [(&metadata_only_policy, true), (&deny_all_policy, false)] {
|
||||
let resolved = builder
|
||||
.clone()
|
||||
.build_with_environment_authority(|_| McpEnvironmentAuthority::Restricted(policy));
|
||||
assert_eq!(
|
||||
resolved
|
||||
.server("selected")
|
||||
.expect("selected plugin")
|
||||
.config()
|
||||
.enabled,
|
||||
expected
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,7 +27,6 @@ use std::sync::OnceLock;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::McpServerSource;
|
||||
use crate::binding::call_tool_result_from_rmcp;
|
||||
use crate::elicitation::ElicitationRequestManager;
|
||||
use crate::elicitation::ElicitationRequestRouter;
|
||||
@@ -276,16 +275,14 @@ impl McpConnectionSet {
|
||||
.into_iter()
|
||||
.filter(|(_, server)| server.enabled())
|
||||
{
|
||||
let is_host_owned_codex_apps = server_name == CODEX_APPS_MCP_SERVER_NAME
|
||||
&& config.mcp_server_catalog.server(&server_name).is_some_and(
|
||||
|server| match server.source() {
|
||||
McpServerSource::Compatibility { .. } => true,
|
||||
McpServerSource::Extension { id } => id == "hosted_plugin_runtime",
|
||||
McpServerSource::Plugin(_)
|
||||
| McpServerSource::SelectedPlugin(_)
|
||||
| McpServerSource::Config => false,
|
||||
},
|
||||
);
|
||||
let is_host_owned_codex_apps = config
|
||||
.mcp_server_catalog
|
||||
.server(&server_name)
|
||||
.is_some_and(|server| {
|
||||
server
|
||||
.source()
|
||||
.is_host_owned_apps(&server_name, server.config())
|
||||
});
|
||||
let catalog_item_limit = if is_host_owned_codex_apps {
|
||||
MAX_CODEX_APPS_TOOL_CATALOG_ITEMS
|
||||
} else {
|
||||
|
||||
@@ -32,6 +32,7 @@ pub type CodexAppsToolsCache = codex_connectors::ConnectorRuntimeManager<ToolInf
|
||||
pub type CodexAppsToolsCacheKey = codex_connectors::ConnectorRuntimeContextKey;
|
||||
|
||||
pub use catalog::McpCatalogBuilder;
|
||||
pub use catalog::McpEnvironmentAuthority;
|
||||
pub use catalog::McpPluginAttribution;
|
||||
pub use catalog::McpServerConflict;
|
||||
pub use catalog::McpServerConflictAction;
|
||||
|
||||
@@ -541,6 +541,12 @@ pub fn apply_http_headers_helper(
|
||||
config: &codex_config::McpServerConfig,
|
||||
local_process_cwd: PathBuf,
|
||||
) -> Result<Arc<dyn HttpClient>, String> {
|
||||
if matches!(
|
||||
config.disabled_reason,
|
||||
Some(McpServerDisabledReason::Requirements { .. })
|
||||
) {
|
||||
return Err("the MCP server is disabled by managed requirements".to_string());
|
||||
}
|
||||
let codex_config::McpServerTransportConfig::StreamableHttp {
|
||||
url,
|
||||
http_headers_helper: Some(command),
|
||||
@@ -549,12 +555,6 @@ pub fn apply_http_headers_helper(
|
||||
else {
|
||||
return Ok(client);
|
||||
};
|
||||
if matches!(
|
||||
config.disabled_reason,
|
||||
Some(McpServerDisabledReason::Requirements { .. })
|
||||
) {
|
||||
return Err("the MCP server is disabled by managed requirements".to_string());
|
||||
}
|
||||
if !config.is_local_environment() {
|
||||
return Err("HTTP headers helpers can only run in the local environment".to_string());
|
||||
}
|
||||
|
||||
@@ -28,6 +28,7 @@ pub use codex_config::types::UriBasedFileOpener;
|
||||
pub use codex_core::CodexAppsToolsCache;
|
||||
pub use codex_core::CodexThread;
|
||||
pub use codex_core::EnvironmentConfig;
|
||||
pub use codex_core::EnvironmentMcpPolicy;
|
||||
pub use codex_core::EnvironmentNetworkPolicy;
|
||||
pub use codex_core::ForkSnapshot;
|
||||
pub use codex_core::LoadedAgentsMd;
|
||||
|
||||
@@ -346,6 +346,7 @@ fn resolved_local_environments<const N: usize>(
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}),
|
||||
|
||||
@@ -814,6 +814,7 @@ mod tests {
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}
|
||||
@@ -988,6 +989,7 @@ url = "ws://127.0.0.1:8765"
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
};
|
||||
@@ -1223,6 +1225,7 @@ url = "ws://127.0.0.1:8765"
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
};
|
||||
@@ -1559,6 +1562,7 @@ url = "ws://127.0.0.1:8765"
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
};
|
||||
@@ -1605,6 +1609,7 @@ url = "ws://127.0.0.1:8765"
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![root("parent-root")],
|
||||
};
|
||||
|
||||
@@ -37,6 +37,7 @@ pub use codex_network_proxy::NetworkDomainPermissionEntry;
|
||||
pub use codex_network_proxy::NetworkDomainPermissions;
|
||||
pub use codex_network_proxy::NetworkUnixSocketPermission;
|
||||
pub use codex_network_proxy::NetworkUnixSocketPermissions;
|
||||
pub use codex_protocol::mcp_policy::EnvironmentMcpPolicy;
|
||||
pub use codex_protocol::protocol::EnvironmentConfig;
|
||||
pub use codex_thread::BackgroundTerminalInfo;
|
||||
pub use codex_thread::CodexThread;
|
||||
|
||||
+70
-24
@@ -2,6 +2,8 @@ use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::environment_selection::ThreadEnvironments;
|
||||
use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_connectors::ConnectorRuntimeManager;
|
||||
use codex_connectors::ConnectorSnapshot;
|
||||
@@ -19,6 +21,7 @@ use codex_login::CodexAuth;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::EffectiveMcpServer;
|
||||
use codex_mcp::McpConfig;
|
||||
use codex_mcp::McpEnvironmentAuthority;
|
||||
use codex_mcp::McpPluginAttribution;
|
||||
use codex_mcp::McpServerRegistration;
|
||||
use codex_mcp::McpToolCatalogCache;
|
||||
@@ -28,7 +31,9 @@ use codex_mcp::configured_mcp_servers;
|
||||
use codex_mcp::effective_mcp_servers;
|
||||
use codex_plugin::AppConnectorId;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
use codex_protocol::protocol::EnvironmentConfigState;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
|
||||
const LEGACY_CODEX_APPS_REGISTRATION_ID: &str = "legacy_codex_apps";
|
||||
|
||||
@@ -40,18 +45,23 @@ pub(crate) struct McpRuntimeProjection {
|
||||
pub(crate) selected_plugins: SelectedPluginSnapshot,
|
||||
}
|
||||
|
||||
pub(crate) enum McpEnvironmentScope<'a> {
|
||||
/// Controller-level operations without an associated thread.
|
||||
HostOnly,
|
||||
/// Initial thread selections before the live environment store exists.
|
||||
Initial(&'a [TurnEnvironmentSelection]),
|
||||
/// Current attachment state for an existing thread.
|
||||
Live(&'a ThreadEnvironments),
|
||||
}
|
||||
|
||||
pub(crate) struct McpThreadIdentity<'a> {
|
||||
pub(crate) session_source: &'a SessionSource,
|
||||
pub(crate) originator: &'a str,
|
||||
pub(crate) environments: McpEnvironmentScope<'a>,
|
||||
}
|
||||
|
||||
enum OrderedMcpOverlay {
|
||||
Set {
|
||||
contributor_id: &'static str,
|
||||
contribution_order: usize,
|
||||
name: String,
|
||||
config: Box<McpServerConfig>,
|
||||
},
|
||||
Set(Box<McpServerRegistration>),
|
||||
Remove {
|
||||
contributor_id: &'static str,
|
||||
contribution_order: usize,
|
||||
@@ -107,6 +117,7 @@ impl McpManager {
|
||||
// originator; active-thread tool calls use runtime_config_for_step below.
|
||||
/*originator*/
|
||||
None,
|
||||
McpEnvironmentScope::HostOnly,
|
||||
)
|
||||
.await
|
||||
.config
|
||||
@@ -133,6 +144,7 @@ impl McpManager {
|
||||
)
|
||||
.with_session_source(identity.session_source),
|
||||
Some(identity.originator),
|
||||
identity.environments,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -141,6 +153,7 @@ impl McpManager {
|
||||
&self,
|
||||
context: McpServerContributionContext<'_, Config>,
|
||||
originator: Option<&str>,
|
||||
environment_scope: McpEnvironmentScope<'_>,
|
||||
) -> McpRuntimeProjection {
|
||||
let config = context.config();
|
||||
let mut selected_plugin_available = false;
|
||||
@@ -155,12 +168,23 @@ impl McpManager {
|
||||
for contribution in contributor.contribute(context).await {
|
||||
match contribution {
|
||||
McpServerContribution::Set { name, config } => {
|
||||
overlays.push(OrderedMcpOverlay::Set {
|
||||
contributor_id: contributor.id(),
|
||||
contribution_order,
|
||||
name,
|
||||
config,
|
||||
});
|
||||
overlays.push(OrderedMcpOverlay::Set(Box::new(
|
||||
McpServerRegistration::from_extension(
|
||||
name,
|
||||
contributor.id(),
|
||||
contribution_order,
|
||||
*config,
|
||||
),
|
||||
)));
|
||||
}
|
||||
McpServerContribution::HostedApps { config } => {
|
||||
overlays.push(OrderedMcpOverlay::Set(Box::new(
|
||||
McpServerRegistration::from_hosted_apps(
|
||||
contributor.id(),
|
||||
contribution_order,
|
||||
*config,
|
||||
),
|
||||
)));
|
||||
}
|
||||
McpServerContribution::SelectedPlugin {
|
||||
name,
|
||||
@@ -237,17 +261,7 @@ impl McpManager {
|
||||
|
||||
for overlay in overlays {
|
||||
match overlay {
|
||||
OrderedMcpOverlay::Set {
|
||||
contributor_id,
|
||||
contribution_order,
|
||||
name,
|
||||
config,
|
||||
} => catalog.register(McpServerRegistration::from_extension(
|
||||
name,
|
||||
contributor_id,
|
||||
contribution_order,
|
||||
*config,
|
||||
)),
|
||||
OrderedMcpOverlay::Set(registration) => catalog.register(*registration),
|
||||
OrderedMcpOverlay::Remove {
|
||||
contributor_id,
|
||||
contribution_order,
|
||||
@@ -255,7 +269,39 @@ impl McpManager {
|
||||
} => catalog.remove_extension(name, contributor_id, contribution_order),
|
||||
}
|
||||
}
|
||||
let catalog = catalog.build();
|
||||
let selections = match environment_scope {
|
||||
McpEnvironmentScope::HostOnly => None,
|
||||
McpEnvironmentScope::Initial(selections) => Some(selections.to_vec()),
|
||||
McpEnvironmentScope::Live(environments) => Some(environments.selections()),
|
||||
};
|
||||
let catalog = catalog.build_with_environment_authority(|environment_id| {
|
||||
let Some(selections) = selections.as_ref() else {
|
||||
return McpEnvironmentAuthority::Unrestricted;
|
||||
};
|
||||
let Some(selection) = selections
|
||||
.iter()
|
||||
.find(|selection| selection.environment_id == environment_id)
|
||||
else {
|
||||
return if environment_id == DEFAULT_MCP_SERVER_ENVIRONMENT_ID {
|
||||
McpEnvironmentAuthority::Unrestricted
|
||||
} else {
|
||||
McpEnvironmentAuthority::SelectedPluginsOnly
|
||||
};
|
||||
};
|
||||
|
||||
match &selection.config {
|
||||
EnvironmentConfigState::FromThread => McpEnvironmentAuthority::Unrestricted,
|
||||
EnvironmentConfigState::Pending | EnvironmentConfigState::Failed(_) => {
|
||||
McpEnvironmentAuthority::Unavailable
|
||||
}
|
||||
EnvironmentConfigState::Ready(config) => config
|
||||
.mcp_policy
|
||||
.as_ref()
|
||||
.map_or(McpEnvironmentAuthority::Unrestricted, |policy| {
|
||||
McpEnvironmentAuthority::Restricted(policy)
|
||||
}),
|
||||
}
|
||||
});
|
||||
for conflict in catalog.conflicts() {
|
||||
tracing::warn!(
|
||||
server = conflict.name,
|
||||
|
||||
@@ -66,6 +66,9 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
|
||||
}
|
||||
|
||||
let unprompted_missing = filter_prompted_mcp_dependencies(sess, &missing).await;
|
||||
// Do not prompt for servers that managed or attachment policy would reject.
|
||||
let unprompted_missing =
|
||||
admit_mcp_dependencies(sess, config.as_ref(), unprompted_missing).await;
|
||||
if unprompted_missing.is_empty() {
|
||||
return;
|
||||
}
|
||||
@@ -73,39 +76,54 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
|
||||
if should_install_mcp_dependencies(sess, turn_context, &unprompted_missing, cancellation_token)
|
||||
.await
|
||||
{
|
||||
maybe_install_mcp_dependencies(
|
||||
sess,
|
||||
turn_context,
|
||||
config.as_ref(),
|
||||
mentioned_skills,
|
||||
elicitation_reviewer,
|
||||
)
|
||||
.await;
|
||||
// Policy may have changed while waiting for the installation prompt.
|
||||
let missing = admit_mcp_dependencies(sess, config.as_ref(), unprompted_missing).await;
|
||||
maybe_install_mcp_dependencies(sess, turn_context, missing, elicitation_reviewer).await;
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn maybe_install_mcp_dependencies(
|
||||
// Resolve proposed servers through the existing policy-aware catalog before installing them.
|
||||
async fn admit_mcp_dependencies(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
config: &crate::config::Config,
|
||||
mentioned_skills: &[SkillMetadata],
|
||||
elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
) {
|
||||
if mentioned_skills.is_empty()
|
||||
|| !config
|
||||
.features
|
||||
.enabled(codex_features::Feature::SkillMcpDependencyInstall)
|
||||
{
|
||||
return;
|
||||
mut candidates: HashMap<String, McpServerConfig>,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
if candidates.is_empty() {
|
||||
return candidates;
|
||||
}
|
||||
|
||||
let codex_home = config.codex_home.clone();
|
||||
let installed = sess.runtime_mcp_servers(config).await;
|
||||
let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed);
|
||||
// Stage candidates in memory so global requirements apply before catalog resolution.
|
||||
let mut candidate_config = config.clone();
|
||||
let mut servers = config.mcp_servers.get().clone();
|
||||
servers.extend(candidates.clone());
|
||||
if let Err(err) = candidate_config.mcp_servers.set(servers) {
|
||||
warn!("failed to validate MCP dependencies for mentioned skills: {err}");
|
||||
return HashMap::new();
|
||||
}
|
||||
|
||||
// Keep only the same candidate when attachment policy leaves it enabled.
|
||||
let catalog = sess.runtime_mcp_config(&candidate_config).await;
|
||||
candidates.retain(|name, config| {
|
||||
catalog
|
||||
.mcp_server_catalog
|
||||
.server(name)
|
||||
.is_some_and(|server| server.config().enabled && server.config() == config)
|
||||
});
|
||||
candidates
|
||||
}
|
||||
|
||||
async fn maybe_install_mcp_dependencies(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
missing: HashMap<String, McpServerConfig>,
|
||||
elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
) {
|
||||
if missing.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let config = turn_context.config.as_ref();
|
||||
let codex_home = config.codex_home.clone();
|
||||
let mut servers = match load_global_mcp_servers(&codex_home).await {
|
||||
Ok(servers) => servers,
|
||||
Err(err) => {
|
||||
@@ -114,7 +132,6 @@ pub(crate) async fn maybe_install_mcp_dependencies(
|
||||
}
|
||||
};
|
||||
|
||||
let mut updated = false;
|
||||
let mut added = Vec::new();
|
||||
for (name, config) in missing {
|
||||
if servers.contains_key(&name) {
|
||||
@@ -122,10 +139,9 @@ pub(crate) async fn maybe_install_mcp_dependencies(
|
||||
}
|
||||
servers.insert(name.clone(), config.clone());
|
||||
added.push((name, config));
|
||||
updated = true;
|
||||
}
|
||||
|
||||
if !updated {
|
||||
if added.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -1122,6 +1122,7 @@ async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Resul
|
||||
.snapshot(),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}),
|
||||
|
||||
@@ -128,6 +128,7 @@ impl Session {
|
||||
McpThreadIdentity {
|
||||
session_source: &session_source,
|
||||
originator: &originator,
|
||||
environments: McpEnvironmentScope::Live(&self.services.turn_environments),
|
||||
},
|
||||
&ready_selected_capability_roots,
|
||||
executor_capability_discovery.as_deref(),
|
||||
@@ -200,6 +201,7 @@ impl Session {
|
||||
McpThreadIdentity {
|
||||
session_source: &desired.session_source,
|
||||
originator: &desired.originator,
|
||||
environments: McpEnvironmentScope::Live(&self.services.turn_environments),
|
||||
},
|
||||
&ready_selected_capability_roots,
|
||||
executor_capability_discovery.as_deref(),
|
||||
@@ -254,6 +256,7 @@ impl Session {
|
||||
McpThreadIdentity {
|
||||
session_source: &desired.session_source,
|
||||
originator: &desired.originator,
|
||||
environments: McpEnvironmentScope::Live(&self.services.turn_environments),
|
||||
},
|
||||
&ready_selected_capability_roots,
|
||||
executor_capability_discovery.as_deref(),
|
||||
@@ -645,6 +648,7 @@ impl Session {
|
||||
McpThreadIdentity {
|
||||
session_source: &turn_context.session_source,
|
||||
originator: &turn_context.originator,
|
||||
environments: McpEnvironmentScope::Live(&self.services.turn_environments),
|
||||
},
|
||||
&ready_selected_capability_roots,
|
||||
executor_capability_discovery.as_deref(),
|
||||
|
||||
@@ -264,6 +264,7 @@ pub(crate) struct PreviousTurnSettings {
|
||||
|
||||
use crate::exec_policy::ExecPolicyUpdateError;
|
||||
use crate::guardian::GuardianReviewSessionManager;
|
||||
use crate::mcp::McpEnvironmentScope;
|
||||
use crate::mcp::McpManager;
|
||||
use crate::mcp::McpThreadIdentity;
|
||||
use crate::network_policy_decision::execpolicy_network_rule_amendment;
|
||||
|
||||
@@ -147,6 +147,7 @@ impl SessionConfiguration {
|
||||
permission_profile: self.permission_profile_state.snapshot(),
|
||||
shell_environment_policy: self.shell_environment_policy.clone(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}
|
||||
@@ -893,6 +894,7 @@ impl Session {
|
||||
McpThreadIdentity {
|
||||
session_source: &mcp_session_source,
|
||||
originator: &mcp_originator,
|
||||
environments: McpEnvironmentScope::Initial(environment_selections),
|
||||
},
|
||||
/*ready_selected_capability_roots*/ &[],
|
||||
/*executor_capability_discovery*/ None,
|
||||
@@ -1046,7 +1048,8 @@ impl Session {
|
||||
&mcp_projection.config,
|
||||
auth.as_ref(),
|
||||
)
|
||||
.into_keys()
|
||||
.into_iter()
|
||||
.filter_map(|(name, server)| server.enabled().then_some(name))
|
||||
.collect::<Vec<_>>();
|
||||
session_telemetry.conversation_starts(
|
||||
config.model_provider.name.as_str(),
|
||||
@@ -1442,6 +1445,9 @@ impl Session {
|
||||
McpThreadIdentity {
|
||||
session_source: &session_configuration.session_source,
|
||||
originator: &session_configuration.originator,
|
||||
environments: McpEnvironmentScope::Live(
|
||||
&sess.services.turn_environments,
|
||||
),
|
||||
},
|
||||
/*ready_selected_capability_roots*/ &[],
|
||||
/*executor_capability_discovery*/ None,
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::agent::control::SpawnAgentOptions;
|
||||
use crate::config::test_config;
|
||||
use crate::init_state_db;
|
||||
use crate::installation_id::INSTALLATION_ID_FILENAME;
|
||||
use crate::mcp::McpEnvironmentScope;
|
||||
use crate::mcp::McpThreadIdentity;
|
||||
use crate::rollout::RolloutRecorder;
|
||||
use crate::session::session::SessionSettingsUpdate;
|
||||
@@ -1031,6 +1032,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
McpThreadIdentity {
|
||||
session_source: &SessionSource::Exec,
|
||||
originator: &first_originator,
|
||||
environments: McpEnvironmentScope::Live(&first_session.services.turn_environments),
|
||||
},
|
||||
/*ready_selected_capability_roots*/ &[],
|
||||
/*executor_capability_discovery*/ None,
|
||||
@@ -1048,6 +1050,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
McpThreadIdentity {
|
||||
session_source: &second_session_source,
|
||||
originator: &second_originator,
|
||||
environments: McpEnvironmentScope::Live(&second_session.services.turn_environments),
|
||||
},
|
||||
/*ready_selected_capability_roots*/ &[],
|
||||
/*executor_capability_discovery*/ None,
|
||||
|
||||
@@ -140,6 +140,7 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
),
|
||||
shell_environment_policy: selected_shell_environment_policy.clone(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}),
|
||||
|
||||
@@ -32,6 +32,7 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context::
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}),
|
||||
|
||||
@@ -29,6 +29,7 @@ async fn approval_key_uses_path_uri_and_includes_environment_id() {
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}),
|
||||
|
||||
@@ -545,6 +545,7 @@ mod tests {
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}),
|
||||
|
||||
@@ -918,6 +918,7 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava
|
||||
.snapshot(),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
},
|
||||
|
||||
@@ -635,6 +635,7 @@ async fn environment_command_restrictions_override_saved_prefix_approvals() -> R
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::Disabled),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: Some(RequirementsExecPolicy::new(invalid_policy)),
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
},
|
||||
@@ -657,6 +658,7 @@ async fn environment_command_restrictions_override_saved_prefix_approvals() -> R
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::Disabled),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: Some(RequirementsExecPolicy::new(environment_policy)),
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
},
|
||||
@@ -760,6 +762,7 @@ async fn environment_command_policy_changes_invalidate_session_approvals() -> Re
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: Some(RequirementsExecPolicy::new(policy)),
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
},
|
||||
|
||||
@@ -747,6 +747,7 @@ async fn guardian_session_is_reused_for_consecutive_tool_reviews_without_prewarm
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
};
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
use codex_core::EnvironmentConfig;
|
||||
use codex_core::EnvironmentMcpPolicy;
|
||||
use codex_core::TurnInputRequest;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -9,8 +12,12 @@ use codex_config::McpServerOAuthConfig;
|
||||
use codex_config::McpServerTransportConfig;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::mcp_policy::McpServerIdentity;
|
||||
use codex_protocol::mcp_policy::McpServerRequirement;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::PermissionProfileSnapshot;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EnvironmentConfigState;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
@@ -43,6 +50,8 @@ const SKILL_TEST_NAME: &str =
|
||||
"suite::mcp_startup_refresh_http_proxy::skill_mcp_dependency_oauth_uses_configured_http_client";
|
||||
const SERVER_NAME: &str = "proxied_mcp";
|
||||
const SERVER_URL: &str = "http://mcp-proxy.invalid/api/codex/ps/mcp";
|
||||
const DENIED_SERVER_NAME: &str = "denied_mcp";
|
||||
const DENIED_SERVER_URL: &str = "http://mcp-proxy.invalid/denied-mcp";
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn local_mcp_startup_and_refresh_use_configured_http_client() -> Result<()> {
|
||||
@@ -271,6 +280,12 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
|
||||
.expect("OAuth client registration redirect URI"),
|
||||
)?;
|
||||
assert_eq!(redirect_uri.port(), Some(skill_callback_port));
|
||||
assert!(
|
||||
requests
|
||||
.iter()
|
||||
.all(|request| request.url.path() != "/denied-mcp"),
|
||||
"denied skill dependency must not perform OAuth discovery: {requests:#?}"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -308,7 +323,7 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
|
||||
)
|
||||
.await?;
|
||||
let metadata = format!(
|
||||
"dependencies:\n tools:\n - type: mcp\n value: {SERVER_NAME}\n transport: streamable_http\n url: {SERVER_URL}\n oauth:\n callbackPort: {skill_callback_port}\n"
|
||||
"dependencies:\n tools:\n - type: mcp\n value: {SERVER_NAME}\n transport: streamable_http\n url: {SERVER_URL}\n oauth:\n callbackPort: {skill_callback_port}\n - type: mcp\n value: {DENIED_SERVER_NAME}\n transport: streamable_http\n url: {DENIED_SERVER_URL}\n"
|
||||
);
|
||||
fs.write_file(
|
||||
&PathUri::from_host_native_path(agents_dir.join("openai.yaml"))?,
|
||||
@@ -342,6 +357,26 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
|
||||
});
|
||||
let (sandbox_policy, permission_profile) =
|
||||
turn_permission_fields(PermissionProfile::Disabled, fixture.config.cwd.as_path());
|
||||
let mut environments = local_selections(fixture.config.cwd.clone());
|
||||
environments.environments[0].config = EnvironmentConfigState::Ready(EnvironmentConfig {
|
||||
allow_login_shell: fixture.config.permissions.allow_login_shell,
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::Disabled),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: Some(EnvironmentMcpPolicy {
|
||||
servers: Some(BTreeMap::from([(
|
||||
SERVER_NAME.to_string(),
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: SERVER_URL.to_string(),
|
||||
},
|
||||
},
|
||||
)])),
|
||||
plugins: None,
|
||||
}),
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
});
|
||||
fixture
|
||||
.codex
|
||||
.start_or_steer_turn(
|
||||
@@ -356,7 +391,7 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
|
||||
},
|
||||
])
|
||||
.with_thread_settings(ThreadSettingsOverrides {
|
||||
environments: Some(local_selections(fixture.config.cwd.clone())),
|
||||
environments: Some(environments),
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
sandbox_policy: Some(sandbox_policy),
|
||||
permission_profile,
|
||||
@@ -379,5 +414,6 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
|
||||
callback_port: Some(skill_callback_port),
|
||||
})
|
||||
);
|
||||
assert!(!servers.contains_key(DENIED_SERVER_NAME));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -154,12 +154,19 @@ impl McpServerContributor<Config> for AppsMcpServerContributor {
|
||||
{
|
||||
root_resolved.add_permits(1);
|
||||
}
|
||||
let config = serde_json::from_value(json!({ "url": self.url }))
|
||||
.expect("test Apps MCP server config should be valid");
|
||||
vec![McpServerContribution::Set {
|
||||
name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
config: Box::new(config),
|
||||
}]
|
||||
let config = Box::new(
|
||||
serde_json::from_value(json!({ "url": self.url }))
|
||||
.expect("test Apps MCP server config should be valid"),
|
||||
);
|
||||
let contribution = if self.id == "hosted_plugin_runtime" {
|
||||
McpServerContribution::HostedApps { config }
|
||||
} else {
|
||||
McpServerContribution::Set {
|
||||
name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
config,
|
||||
}
|
||||
};
|
||||
vec![contribution]
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -485,6 +492,7 @@ async fn root_reconciliation_reuses_pending_apps_startup() -> Result<()> {
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![SelectedCapabilityRoot {
|
||||
id: "calendar-root".to_string(),
|
||||
|
||||
@@ -88,7 +88,7 @@ fn sample_plugin_root(home: &TempDir) -> std::path::PathBuf {
|
||||
home.path().join("plugins/cache/test/sample/local")
|
||||
}
|
||||
|
||||
fn write_sample_plugin_manifest_and_config(home: &TempDir) -> std::path::PathBuf {
|
||||
pub(super) fn write_sample_plugin_manifest_and_config(home: &TempDir) -> std::path::PathBuf {
|
||||
write_sample_plugin_manifest_and_config_at_root(
|
||||
home,
|
||||
sample_plugin_root(home),
|
||||
|
||||
@@ -554,6 +554,7 @@ async fn environment_permissions_follow_configuration_ownership() -> Result<()>
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
}),
|
||||
@@ -1178,6 +1179,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment(
|
||||
permission_profile: permission_profile.clone(),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![root("duplicate"), root("duplicate")],
|
||||
}),
|
||||
@@ -1215,6 +1217,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment(
|
||||
permission_profile: permission_profile.clone(),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![root("startup-root"), root("second-root")],
|
||||
}),
|
||||
@@ -1236,6 +1239,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment(
|
||||
permission_profile: permission_profile.clone(),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![root("first-root")],
|
||||
}),
|
||||
@@ -1296,6 +1300,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment(
|
||||
permission_profile: permission_profile.clone(),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![root("first-updated-root")],
|
||||
}),
|
||||
@@ -1366,6 +1371,7 @@ async fn owner_network_policy_is_rejected_until_runtime_enforcement_exists() ->
|
||||
),
|
||||
shell_environment_policy: test.config.permissions.shell_environment_policy.clone(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: Some(EnvironmentNetworkPolicy::from_config(
|
||||
&NetworkProxyConfig::default(),
|
||||
/*managed_allowed_domains_only*/ true,
|
||||
@@ -1443,6 +1449,7 @@ async fn pending_attachment_installs_configuration_before_waiting_turn_resumes()
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![root(id)],
|
||||
};
|
||||
@@ -1794,6 +1801,7 @@ async fn ready_before_selection_exposes_remote_tools_and_capability_context_afte
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: vec![ready_root],
|
||||
}),
|
||||
|
||||
@@ -2,6 +2,7 @@ use anyhow::Context as _;
|
||||
use anyhow::ensure;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::ffi::OsStr;
|
||||
use std::ffi::OsString;
|
||||
@@ -22,6 +23,8 @@ use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerEnvVar;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_config::types::OAuthCredentialsStoreMode;
|
||||
use codex_core::EnvironmentConfig;
|
||||
use codex_core::EnvironmentMcpPolicy;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::config::Config;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
@@ -42,8 +45,12 @@ use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::mcp_policy::McpServerIdentity;
|
||||
use codex_protocol::mcp_policy::McpServerRequirement;
|
||||
use codex_protocol::mcp_policy::PluginMcpRequirements;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::PermissionProfileSnapshot;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
@@ -53,6 +60,7 @@ use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EnvironmentConfigState;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::McpInvocation;
|
||||
use codex_protocol::protocol::McpStartupFailureReason;
|
||||
@@ -60,6 +68,8 @@ use codex_protocol::protocol::McpStartupStatus;
|
||||
use codex_protocol::protocol::McpToolCallBeginEvent;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_cargo_bin::cargo_bin;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -74,6 +84,7 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_no_remote_env;
|
||||
use core_test_support::skip_if_wine_exec;
|
||||
use core_test_support::stdio_server_bin;
|
||||
use core_test_support::submit_thread_settings;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_codex::turn_permission_fields;
|
||||
@@ -568,6 +579,178 @@ async fn mcp_namespace_instructions_are_preserved_without_hiding_tools() -> anyh
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(false; "configured servers")]
|
||||
#[test_case(true; "plugin servers")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn environment_mcp_policy_filters_runtime_config_and_model_tools(
|
||||
from_plugin: bool,
|
||||
) -> anyhow::Result<()> {
|
||||
skip_if_wine_exec!(
|
||||
Ok(()),
|
||||
"requires a Windows test_stdio_server in the Wine-exec environment"
|
||||
);
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let response = mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_assistant_message("msg-1", "done"),
|
||||
responses::ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let command = remote_aware_stdio_server_bin()?;
|
||||
let allowed_command = command.clone();
|
||||
let codex_home = Arc::new(tempdir()?);
|
||||
if from_plugin {
|
||||
let plugin_root =
|
||||
super::plugins::write_sample_plugin_manifest_and_config(codex_home.as_ref());
|
||||
let plugin_server = json!({
|
||||
"command": command,
|
||||
"environment_id": remote_aware_environment_id(),
|
||||
});
|
||||
fs::write(
|
||||
plugin_root.join(".mcp.json"),
|
||||
serde_json::to_vec(&json!({
|
||||
"mcpServers": {
|
||||
"allowed": plugin_server,
|
||||
"blocked": plugin_server,
|
||||
},
|
||||
}))?,
|
||||
)?;
|
||||
}
|
||||
let fixture = test_codex()
|
||||
.with_home(codex_home)
|
||||
.with_model_info_override("gpt-5.4", |model| model.supports_search_tool = false)
|
||||
.with_config(move |config| {
|
||||
if !from_plugin {
|
||||
for server_name in ["allowed", "blocked"] {
|
||||
insert_mcp_server(
|
||||
config,
|
||||
server_name,
|
||||
stdio_transport(command.clone(), /*env*/ None, Vec::new()),
|
||||
TestMcpServerOptions {
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
insert_mcp_server(
|
||||
config,
|
||||
"unselected",
|
||||
stdio_transport(command, /*env*/ None, Vec::new()),
|
||||
TestMcpServerOptions {
|
||||
environment_id: "unselected-environment".to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
|
||||
let selection = fixture
|
||||
.codex
|
||||
.environment_selections()
|
||||
.await
|
||||
.into_iter()
|
||||
.next()
|
||||
.expect("thread should select its executor environment");
|
||||
submit_thread_settings(
|
||||
&fixture.codex,
|
||||
ThreadSettingsOverrides {
|
||||
environments: Some(TurnEnvironmentSelections::new(
|
||||
fixture.config.cwd.clone(),
|
||||
vec![TurnEnvironmentSelection {
|
||||
config: EnvironmentConfigState::Pending,
|
||||
..selection.clone()
|
||||
}],
|
||||
)),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
let (pending_config, _) = fixture.codex.current_mcp_config_and_runtime_context().await;
|
||||
let pending_servers = pending_config.mcp_server_catalog.configured_servers();
|
||||
assert!(!pending_servers["allowed"].enabled);
|
||||
assert!(!pending_servers["unselected"].enabled);
|
||||
|
||||
let allowed_servers = BTreeMap::from([(
|
||||
"allowed".to_string(),
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: allowed_command,
|
||||
},
|
||||
},
|
||||
)]);
|
||||
let mcp_policy = if from_plugin {
|
||||
EnvironmentMcpPolicy {
|
||||
servers: None,
|
||||
plugins: Some(BTreeMap::from([(
|
||||
"sample@test".to_string(),
|
||||
PluginMcpRequirements {
|
||||
mcp_servers: Some(allowed_servers),
|
||||
},
|
||||
)])),
|
||||
}
|
||||
} else {
|
||||
EnvironmentMcpPolicy {
|
||||
servers: Some(allowed_servers),
|
||||
plugins: None,
|
||||
}
|
||||
};
|
||||
|
||||
fixture
|
||||
.codex
|
||||
.environment_ready(
|
||||
&selection,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: PermissionProfileSnapshot::legacy(
|
||||
fixture.config.permissions.permission_profile().clone(),
|
||||
),
|
||||
shell_environment_policy: Default::default(),
|
||||
exec_policy: None,
|
||||
mcp_policy: Some(mcp_policy),
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
let (runtime_config, _) = fixture.codex.current_mcp_config_and_runtime_context().await;
|
||||
let runtime_servers = runtime_config.mcp_server_catalog.configured_servers();
|
||||
assert!(!runtime_servers["blocked"].enabled);
|
||||
assert!(!runtime_servers["unselected"].enabled);
|
||||
fixture
|
||||
.codex
|
||||
.call_mcp_tool(
|
||||
"allowed",
|
||||
"echo",
|
||||
Some(json!({ "message": "ready" })),
|
||||
/*meta*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
fixture
|
||||
.submit_text_turn("show the available MCP tools")
|
||||
.await?;
|
||||
let body = response.single_request().body_json();
|
||||
assert!(responses::namespace_child_tool(&body, "mcp__allowed", "echo").is_some());
|
||||
assert!(responses::namespace_child_tool(&body, "mcp__blocked", "echo").is_none());
|
||||
|
||||
fixture
|
||||
.codex
|
||||
.environment_failed(&selection, "environment policy unavailable".to_string())
|
||||
.await?;
|
||||
let (failed_config, _) = fixture.codex.current_mcp_config_and_runtime_context().await;
|
||||
let failed_servers = failed_config.mcp_server_catalog.configured_servers();
|
||||
assert!(!failed_servers["allowed"].enabled);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
#[serial(mcp_test_value)]
|
||||
async fn stdio_server_round_trip() -> anyhow::Result<()> {
|
||||
|
||||
@@ -356,6 +356,7 @@ async fn exec_command_uses_installed_environment_shell_policy_with_explicit_over
|
||||
..Default::default()
|
||||
},
|
||||
exec_policy: None,
|
||||
mcp_policy: None,
|
||||
network_policy: None,
|
||||
selected_capability_roots: Vec::new(),
|
||||
},
|
||||
|
||||
@@ -133,6 +133,8 @@ pub enum McpServerContribution {
|
||||
name: String,
|
||||
config: Box<McpServerConfig>,
|
||||
},
|
||||
/// Registers the controller-owned Apps server under its reserved name.
|
||||
HostedApps { config: Box<McpServerConfig> },
|
||||
/// Registers a server declared by a plugin selected for this thread.
|
||||
SelectedPlugin {
|
||||
name: String,
|
||||
|
||||
@@ -27,8 +27,7 @@ impl McpServerContributor<Config> for HostedPluginRuntimeExtension {
|
||||
return vec![McpServerContribution::Remove { name }];
|
||||
}
|
||||
|
||||
vec![McpServerContribution::Set {
|
||||
name,
|
||||
vec![McpServerContribution::HostedApps {
|
||||
config: Box::new(hosted_plugin_runtime_mcp_server_config(
|
||||
&config.chatgpt_base_url,
|
||||
config.apps_mcp_product_sku.as_deref(),
|
||||
|
||||
@@ -31,7 +31,7 @@ async fn hosted_plugin_runtime_forwards_thread_originator() -> Result<(), Box<dy
|
||||
/*executor_capability_discovery*/ None,
|
||||
))
|
||||
.await;
|
||||
let [McpServerContribution::Set { config: server, .. }] = contributions.as_slice() else {
|
||||
let [McpServerContribution::HostedApps { config: server }] = contributions.as_slice() else {
|
||||
panic!("hosted plugin runtime should contribute one server");
|
||||
};
|
||||
let McpServerTransportConfig::StreamableHttp { http_headers, .. } = &server.transport else {
|
||||
|
||||
@@ -304,7 +304,9 @@ async fn selected_plugin_contributions(
|
||||
enabled: config.enabled,
|
||||
}),
|
||||
McpServerContribution::SelectedPluginPackage { .. } => None,
|
||||
McpServerContribution::Set { .. } | McpServerContribution::Remove { .. } => {
|
||||
McpServerContribution::Set { .. }
|
||||
| McpServerContribution::HostedApps { .. }
|
||||
| McpServerContribution::Remove { .. } => {
|
||||
panic!("expected selected plugin contribution")
|
||||
}
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use crate::capabilities::SelectedCapabilityRoot;
|
||||
use crate::config_types::ShellEnvironmentPolicy;
|
||||
use crate::mcp_policy::EnvironmentMcpPolicy;
|
||||
use crate::models::PermissionProfileSnapshot;
|
||||
use codex_execpolicy::RequirementsExecPolicy;
|
||||
use codex_network_proxy::EnvironmentNetworkPolicy;
|
||||
@@ -29,6 +30,8 @@ pub struct EnvironmentConfig {
|
||||
pub shell_environment_policy: ShellEnvironmentPolicy,
|
||||
/// Additional managed command restrictions for this environment attachment.
|
||||
pub exec_policy: Option<RequirementsExecPolicy>,
|
||||
/// Additional managed MCP restrictions for this environment attachment.
|
||||
pub mcp_policy: Option<EnvironmentMcpPolicy>,
|
||||
/// Owner-provided traffic restrictions. `None` keeps the existing controller policy.
|
||||
/// Core rejects `Some` until attachment-owned network enforcement is implemented.
|
||||
pub network_policy: Option<EnvironmentNetworkPolicy>,
|
||||
@@ -44,6 +47,7 @@ impl std::fmt::Debug for EnvironmentConfig {
|
||||
.field("permission_profile", &self.permission_profile)
|
||||
.field("shell_environment_policy", &"<redacted>")
|
||||
.field("exec_policy", &self.exec_policy)
|
||||
.field("mcp_policy", &self.mcp_policy)
|
||||
.field("network_policy", &self.network_policy)
|
||||
.field("selected_capability_roots", &self.selected_capability_roots)
|
||||
.finish()
|
||||
|
||||
@@ -1,6 +1,13 @@
|
||||
use serde::Deserialize;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
/// Additional managed MCP restrictions supplied by an environment owner.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct EnvironmentMcpPolicy {
|
||||
pub servers: Option<BTreeMap<String, McpServerRequirement>>,
|
||||
pub plugins: Option<BTreeMap<String, PluginMcpRequirements>>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
|
||||
#[serde(untagged)]
|
||||
pub enum McpServerIdentity {
|
||||
|
||||
Reference in New Issue
Block a user