openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-10 21:00:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Honor permission grant scopes in executor capability discovery (#60461)

Browse Source 
## Why

Approved filesystem access should make executor skills discoverable and readable for the lifetime of the grant. Turn-scoped access must not authorize persistent plugin servers or hooks, or bypass disabled-plugin policy.

## What changed

- Apply environment-specific session grants to capability discovery and refresh MCP after session grants change.
- Use a separate discovery inventory for skills with turn-scoped grants, keeping plugin servers and hooks on the session inventory. Reuse discovery results across unchanged model steps.
- Preserve disabled-plugin checks for unreadable manifests and symlinked manifest files or directories, including ancestor markers, while retaining standalone skill roots.

## Testing

Add coverage for V1 and V2 skill discovery before approval, after approval, and on the next turn; disabled-plugin ownership checks; discovery cache reuse; and session hooks surviving turn-scoped discovery failure.

GitOrigin-RevId: 929a7f8cb01f4bcfb4bee2c50ae02cd762066b2d
This commit is contained in:
Dmitriy Kovalenko
2026-11-09 22:20:07 +00:00
committed by copyberry
parent 0598a96e93
commit 76dcb8199b
15 changed files with 1117 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core-plugins/src/executor_provider.rs
+82 -35
View File
@@ -17,6 +17,7 @@ use codex_utils_path_uri::PathUri;
use codex_utils_path_uri::PathUriParseError;
use codex_utils_plugins::DISCOVERABLE_PLUGIN_MANIFEST_PATHS;
use futures::StreamExt;
use futures::TryStreamExt;
use std::io;
use std::sync::Arc;
use thiserror::Error;
@@ -152,9 +153,6 @@ impl ExecutorPluginProvider {
{
return Ok(PluginRootOwnership::Plugin);
}
if discovery.warnings.is_empty() {
return Ok(PluginRootOwnership::Standalone);
}
}
let environment = self
.environment_manager
@@ -164,10 +162,14 @@ impl ExecutorPluginProvider {
environment_id: environment_id.clone(),
})?;
let file_system = environment.get_filesystem();
// Warning-free discovery can still omit hidden ancestor manifests.
let manifest = find_manifest(
selected_root,
std::iter::successors(Some(path.clone()), PathUri::parent),
file_system.as_ref(),
GetMetadataOptions {
follow_symlinks: false,
},
sandbox,
)
.await?;
@@ -241,6 +243,7 @@ async fn resolve_plugin_root(
selected_root,
std::iter::once(plugin_root.clone()),
file_system,
GetMetadataOptions::default(),
/*sandbox*/ None,
)
.await?
@@ -285,48 +288,92 @@ async fn resolve_plugin_root(
async fn find_manifest(
selected_root: &SelectedCapabilityRoot,
mut plugin_roots: impl Iterator<Item = PathUri> + Send,
plugin_roots: impl Iterator<Item = PathUri> + Send,
file_system: &dyn ExecutorFileSystem,
metadata_options: GetMetadataOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> Result<Option<PathUri>, ExecutorPluginProviderError> {
let root_id = &selected_root.id;
let mut plugin_root = plugin_roots.next();
let mut marker_index = 0;
let probes = std::iter::from_fn(move || {
let root = plugin_root.clone()?;
let relative_path = *DISCOVERABLE_PLUGIN_MANIFEST_PATHS.get(marker_index)?;
let candidate_uri = root.join(relative_path).map_err(|source| {
ExecutorPluginProviderError::InvalidManifestPath {
root_id: root_id.clone(),
root,
relative_path,
source,
let roots = futures::stream::iter(plugin_roots)
.map(|root| async move {
// Keep lexical ancestors, but resolve aliases before no-follow marker probes.
// Native anchors need no resolution; `file:///C:` converts to a drive-relative path.
if metadata_options.follow_symlinks || root.parent().is_none() {
Ok(root)
} else {
file_system
.canonicalize(&root, sandbox)
.await
.map_err(|source| ExecutorPluginProviderError::InspectRoot {
root_id: root_id.clone(),
path: root.clone(),
source,
})
}
});
marker_index += 1;
if marker_index == DISCOVERABLE_PLUGIN_MANIFEST_PATHS.len() {
marker_index = 0;
plugin_root = plugin_roots.next();
}
Some(candidate_uri)
});
})
.buffered(MAX_CONCURRENT_MANIFEST_PROBES);
let probes = roots
.map_ok(|root| {
futures::stream::iter(
(0..DISCOVERABLE_PLUGIN_MANIFEST_PATHS.len())
.map(move |index| Ok((root.clone(), index))),
)
})
.try_flatten();
// Pipeline remote metadata calls while consuming results in ancestor/manifest priority order.
let mut results = futures::stream::iter(probes)
.map(|candidate_uri| async move {
let candidate_uri = candidate_uri?;
let mut results = probes
.map(|probe| async move {
let (root, marker_index) = probe?;
let relative_path = DISCOVERABLE_PLUGIN_MANIFEST_PATHS[marker_index];
let candidate_uri = root.join(relative_path).map_err(|source| {
ExecutorPluginProviderError::InvalidManifestPath {
root_id: root_id.clone(),
root,
relative_path,
source,
}
})?;
match file_system
.get_metadata(&candidate_uri, GetMetadataOptions::default(), sandbox)
.get_metadata(&candidate_uri, metadata_options, sandbox)
.await
{
Ok(metadata) if metadata.is_file => Ok(Some(candidate_uri)),
Ok(_) => Ok(None),
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(None),
Err(source) => Err(ExecutorPluginProviderError::InspectManifest {
root_id: root_id.clone(),
path: candidate_uri,
source,
}),
Ok(metadata)
if metadata.is_file
|| (!metadata_options.follow_symlinks && metadata.is_symlink) =>
{
return Ok(Some(candidate_uri));
}
Ok(_) => {}
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
Err(source) => {
return Err(ExecutorPluginProviderError::InspectManifest {
root_id: root_id.clone(),
path: candidate_uri,
source,
});
}
}
// A symlinked marker directory can hide its manifest, including in an ancestor.
if !metadata_options.follow_symlinks
&& let Some(directory) = candidate_uri.parent()
{
match file_system
.get_metadata(&directory, metadata_options, sandbox)
.await
{
Ok(metadata) if metadata.is_symlink => return Ok(Some(candidate_uri)),
Ok(_) => {}
Err(error) if error.kind() == io::ErrorKind::NotFound => {}
Err(source) => {
return Err(ExecutorPluginProviderError::InspectManifest {
root_id: root_id.clone(),
path: directory,
source,
});
}
}
}
Ok(None)
})
.buffered(MAX_CONCURRENT_MANIFEST_PROBES);
while let Some(result) = results.next().await {
codex-rs/core-plugins/src/executor_provider_tests.rs
+49 -36
View File
@@ -250,6 +250,13 @@ async fn root_ownership_survives_malformed_ancestor_manifests() {
let directory = tempdir().expect("tempdir");
let root = directory.path().join("package/skills");
fs::create_dir_all(&root).expect("create skills root");
#[cfg(unix)]
let root = {
let alias = directory.path().join("owning-package/skills");
fs::create_dir_all(alias.parent().unwrap()).expect("create owning package");
std::os::unix::fs::symlink(&root, &alias).expect("symlink skills");
alias
};
let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests()));
let selected = selected_root("example@marketplace", LOCAL_ENVIRONMENT_ID, &root);
assert_eq!(
@@ -300,9 +307,8 @@ async fn root_ownership_survives_malformed_ancestor_manifests() {
async fn discovery_ownership_uses_ancestors_not_nested_plugins() {
let directory = tempdir().expect("tempdir");
let path = PathUri::from_host_native_path(directory.path()).unwrap();
let selected = selected_root_uri("example@marketplace", "executor", path.clone());
let provider =
ExecutorPluginProvider::new(Arc::new(environment_manager_without_environments()));
let selected = selected_root_uri("example@marketplace", LOCAL_ENVIRONMENT_ID, path.clone());
let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests()));
let mut discovery = CapabilityRootDiscovery {
id: selected.id.clone(),
path: path.clone(),
@@ -330,6 +336,8 @@ async fn discovery_ownership_uses_ancestors_not_nested_plugins() {
.unwrap(),
PluginRootOwnership::Plugin
);
let provider =
ExecutorPluginProvider::new(Arc::new(environment_manager_without_environments()));
assert!(matches!(
provider
.root_ownership(&selected, /*discovery*/ None, /*sandbox*/ None)
@@ -377,6 +385,7 @@ async fn ancestor_manifest_probes_pipeline_and_preserve_error_order() {
&selected,
std::iter::successors(Some(root.clone()), PathUri::parent),
&file_system,
GetMetadataOptions::default(),
/*sandbox*/ None,
));
assert!(futures::poll!(resolution.as_mut()).is_pending());
@@ -407,42 +416,46 @@ async fn ancestor_manifest_probes_pipeline_and_preserve_error_order() {
#[tokio::test]
async fn manifest_presence_uses_sandboxed_metadata_without_reading_contents() {
let directory = tempdir().expect("tempdir");
let plugin_root = PathUri::from_host_native_path(directory.path()).expect("root URI");
let plugin_root = PathUri::parse("file:///C:").expect("Windows anchor URI");
let manifest_path = plugin_root
.join(".codex-plugin/plugin.json")
.expect("manifest URI");
let file_system = SyntheticPluginFileSystem {
plugin_root: plugin_root.clone(),
manifest_path: manifest_path.clone(),
calls: Mutex::new(Vec::new()),
metadata_gate: None,
metadata_delay: None,
metadata_error: None,
};
let sandbox =
FileSystemSandboxContext::from_permission_profile(Default::default(), plugin_root.clone());
assert_eq!(
find_manifest(
&selected_root_uri("selected-demo", "executor-test", plugin_root.clone()),
std::iter::once(plugin_root.clone()),
&file_system,
Some(&sandbox),
)
.await
.expect("probe manifest"),
Some(manifest_path.clone()),
);
assert_eq!(
*file_system
.calls
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner),
vec![FileSystemCall::SandboxedMetadata {
path: manifest_path,
sandbox: Box::new(sandbox)
}],
);
for follow_symlinks in [true, false] {
let file_system = SyntheticPluginFileSystem {
plugin_root: plugin_root.clone(),
manifest_path: manifest_path.clone(),
calls: Mutex::new(Vec::new()),
metadata_gate: None,
metadata_delay: None,
metadata_error: None,
};
let sandbox = FileSystemSandboxContext::from_permission_profile(
Default::default(),
plugin_root.clone(),
);
assert_eq!(
find_manifest(
&selected_root_uri("selected-demo", "executor-test", plugin_root.clone()),
std::iter::once(plugin_root.clone()),
&file_system,
GetMetadataOptions { follow_symlinks },
Some(&sandbox),
)
.await
.expect("probe manifest"),
Some(manifest_path.clone()),
);
assert_eq!(
*file_system
.calls
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner),
vec![FileSystemCall::SandboxedMetadata {
path: manifest_path.clone(),
sandbox: Box::new(sandbox)
}],
);
}
}
#[tokio::test]
codex-rs/core/src/mcp.rs
+4 -3
View File
@@ -250,12 +250,13 @@ impl McpManager {
}
/// Resolves only plugins from this step's ready roots, independent of shared MCP refresh.
pub(crate) async fn selected_plugins_for_step(
pub(crate) async fn selected_plugins_for_step<'a>(
&self,
context: McpServerContributionContext<'_, Config>,
context: McpServerContributionContext<'a, Config>,
plugins_config: &PluginsConfigToml,
disabled_plugin_ids: &[String],
disabled_plugin_ids: &'a [String],
) -> SelectedPluginSnapshot {
let context = context.with_disabled_plugin_ids(disabled_plugin_ids);
let mut selected = SelectedPluginSnapshot {
disabled_plugin_ids: disabled_plugin_ids.to_vec(),
..Default::default()
codex-rs/core/src/session/mcp.rs
+57 -11
View File
@@ -109,7 +109,11 @@ impl Session {
let ready_selected_capability_roots =
Self::ready_selected_capability_roots(&selected_capability_roots);
let (executor_capability_discovery_v2, inventory_changed) = self
.executor_capability_discovery_for_step_v2(config, &environments)
.executor_capability_discovery_for_step_v2(
config,
&environments,
/*skill_turn*/ None,
)
.await;
let (executor_capability_discovery, discovery_recovered) = self
.executor_capability_discovery_for_step(
@@ -117,6 +121,7 @@ impl Session {
&selected_capability_roots,
&environments,
executor_capability_discovery_v2.as_deref(),
/*skill_turn*/ None,
)
.await;
if inventory_changed || discovery_recovered {
@@ -229,7 +234,11 @@ impl Session {
let ready_selected_capability_roots =
Self::ready_selected_capability_roots(&selected_capability_roots);
let (executor_capability_discovery_v2, _) = self
.executor_capability_discovery_for_step_v2(&desired.config, &desired.environments)
.executor_capability_discovery_for_step_v2(
&desired.config,
&desired.environments,
/*skill_turn*/ None,
)
.await;
let (executor_capability_discovery, _) = self
.executor_capability_discovery_for_step(
@@ -237,6 +246,7 @@ impl Session {
&selected_capability_roots,
&desired.environments,
executor_capability_discovery_v2.as_deref(),
/*skill_turn*/ None,
)
.await;
let mcp_projection = self
@@ -312,7 +322,11 @@ impl Session {
let ready_selected_capability_roots =
Self::ready_selected_capability_roots(&selected_capability_roots);
let (executor_capability_discovery_v2, _) = self
.executor_capability_discovery_for_step_v2(&desired.config, &desired.environments)
.executor_capability_discovery_for_step_v2(
&desired.config,
&desired.environments,
/*skill_turn*/ None,
)
.await;
let (executor_capability_discovery, _) = self
.executor_capability_discovery_for_step(
@@ -320,6 +334,7 @@ impl Session {
&selected_capability_roots,
&desired.environments,
executor_capability_discovery_v2.as_deref(),
/*skill_turn*/ None,
)
.await;
let mcp_projection = self
@@ -462,6 +477,19 @@ impl Session {
Arc::new(codex_mcp::McpBinding::empty(config))
}
fn discovery_sandbox(
&self,
environment: &super::turn_context::TurnEnvironment,
skill_turn: Option<&TurnContext>,
) -> codex_exec_server::FileSystemSandboxContext {
let id = &environment.selection.environment_id;
let grants = skill_turn.map_or_else(
|| self.services.granted_permissions(id),
|turn| turn.granted_permissions(id),
);
environment.sandbox_context(grants)
}
#[tracing::instrument(
name = "capability_roots.snapshot_for_step",
skip_all,
@@ -473,6 +501,7 @@ impl Session {
resolved_roots: &[ResolvedSelectedCapabilityRoot],
environments: &TurnEnvironmentSnapshot,
v2_snapshots: Option<&[ExecutorCapabilityDiscoveryV2Snapshot]>,
skill_turn: Option<&TurnContext>,
) -> (Option<Arc<ExecutorCapabilityDiscoverySnapshot>>, bool) {
if v2_snapshots.is_some() {
return (None, false);
@@ -506,16 +535,19 @@ impl Session {
.map(|environment| {
(
environment.selection.environment_id.clone(),
environment.sandbox_context(/*additional_permissions*/ None),
self.discovery_sandbox(environment, skill_turn),
)
})
.collect::<HashMap<_, _>>()
} else {
HashMap::new()
};
let cache = self
.services
.thread_extension_data
// Each scope retains one result per root; temporary grants must not evict the
// session inventory on every model step. The turn cache expires with its grants.
let cache = skill_turn
.map_or(&self.services.thread_extension_data, |turn| {
turn.extension_data.as_ref()
})
.get_or_init(ExecutorCapabilityDiscoveryCache::default);
let selected_capability_roots = resolved_roots
.iter()
@@ -540,7 +572,10 @@ impl Session {
let discovery = cache
.snapshot(&selected_capability_roots, &sandbox_contexts)
.await;
(Some(Arc::new(discovery)), cache.take_recovered_discovery())
(
Some(Arc::new(discovery)),
skill_turn.is_none() && cache.take_recovered_discovery(),
)
}
/// Acquires V2 inventories and reports changes without scheduling an MCP refresh.
@@ -548,6 +583,7 @@ impl Session {
&self,
config: &Config,
environments: &TurnEnvironmentSnapshot,
skill_turn: Option<&TurnContext>,
) -> (Option<Vec<ExecutorCapabilityDiscoveryV2Snapshot>>, bool) {
if !config
.features
@@ -588,7 +624,7 @@ impl Session {
}
let request = DiscoverV2CapabilitiesRequest {
cwd: selected.cwd().clone(),
sandbox: Some(selected.sandbox_context(/*additional_permissions*/ None)),
sandbox: Some(self.discovery_sandbox(selected, skill_turn)),
};
let result = cache
.discover(environment_id, selected.environment(), request)
@@ -598,7 +634,12 @@ impl Session {
result,
});
}
if snapshots.is_empty() && skipped_incompatible_executor {
let incompatible = snapshots.is_empty() && skipped_incompatible_executor;
if skill_turn.is_some() {
// Only session discovery updates the inventory used to invalidate MCP.
return ((!incompatible).then_some(snapshots), false);
}
if incompatible {
let changed = self
.services
.thread_extension_data
@@ -871,7 +912,11 @@ impl Session {
let environment_selections = environments.all_selections();
let desired = self.latest_mcp_desired_state(auth, environments).await;
let (executor_capability_discovery_v2, _) = self
.executor_capability_discovery_for_step_v2(&desired.config, &desired.environments)
.executor_capability_discovery_for_step_v2(
&desired.config,
&desired.environments,
/*skill_turn*/ None,
)
.await;
let (executor_capability_discovery, _) = self
.executor_capability_discovery_for_step(
@@ -879,6 +924,7 @@ impl Session {
&selected_capability_roots,
&desired.environments,
executor_capability_discovery_v2.as_deref(),
/*skill_turn*/ None,
)
.await;
let mcp_projection = self
codex-rs/core/src/session/mod.rs
+76 -29
View File
@@ -3816,6 +3816,7 @@ impl Session {
environment_id,
response.permissions.clone().into(),
);
self.mark_mcp_runtime_dirty();
}
}
}
@@ -4365,7 +4366,11 @@ impl Session {
let ready_selected_capability_roots =
Self::ready_selected_capability_roots(&selected_capability_roots);
let (executor_capability_discovery_v2, inventory_changed) = self
.executor_capability_discovery_for_step_v2(&turn_context.config, &environments)
.executor_capability_discovery_for_step_v2(
&turn_context.config,
&environments,
/*skill_turn*/ None,
)
.await;
let v2_snapshots = executor_capability_discovery_v2.as_ref();
let (executor_capability_discovery, discovery_recovered) = if v2_snapshots.is_some() {
@@ -4376,6 +4381,7 @@ impl Session {
&selected_capability_roots,
&environments,
executor_capability_discovery_v2.as_deref(),
/*skill_turn*/ None,
)
.await
};
@@ -4392,14 +4398,37 @@ impl Session {
self.host_skills_for_step(&turn_context, &environments)
.await,
);
if turn_context
.config
.features
.enabled(Feature::SkillCatalogDeltas)
&& let Some(grants) = turn_context.turn_skill_filesystem_grants(&environments)
{
extension_data.insert(grants);
}
// Only skill discovery may use a turn's temporary file access. Plugin servers and
// hooks use the session inventory above because they can outlive the step.
let turn_skill_discovery =
if let Some(grants) = turn_context.turn_skill_filesystem_grants(&environments) {
if turn_context
.config
.features
.enabled(Feature::SkillCatalogDeltas)
{
extension_data.insert(grants);
}
let (v2, _) = self
.executor_capability_discovery_for_step_v2(
&turn_context.config,
&environments,
Some(&turn_context),
)
.await;
let (v1, _) = self
.executor_capability_discovery_for_step(
&turn_context.config,
&selected_capability_roots,
&environments,
v2.as_deref(),
Some(&turn_context),
)
.await;
Some((v1, v2))
} else {
None
};
if let Some(messages) = settings
.model_info
.model_messages
@@ -4462,28 +4491,53 @@ impl Session {
mcp
};
extension_data.insert(selected_capability_roots);
// A step keeps the plugins from the environments it captured, even if shared MCP
// moves on to another environment. Its skill tools and the model use this same copy.
// Policy follows this step's MCP binding, which may have refreshed mid-turn.
let selected_plugins = self
.services
.mcp_manager
.selected_plugins_for_step(
let selected_environments = environments.all_selections();
let select_plugins = |discovery, discovery_v2| {
self.services.mcp_manager.selected_plugins_for_step(
codex_extension_api::McpServerContributionContext::for_step(
turn_context.config.as_ref(),
&self.services.mcp_thread_init,
&self.services.thread_extension_data,
&turn_context.originator,
&ready_selected_capability_roots,
executor_capability_discovery.as_deref(),
executor_capability_discovery_v2.as_deref(),
discovery,
discovery_v2,
)
.with_session_source(&turn_context.session_source)
.with_selected_environments(&environments.all_selections()),
.with_selected_environments(&selected_environments),
&mcp.config().plugins,
&turn_context.disabled_plugin_ids,
)
.await;
};
// Resolve skill ownership from the same inventory as skill discovery.
let (skill_discovery, skill_discovery_v2) = match &turn_skill_discovery {
Some((v1, v2)) => (v1.as_deref(), v2.as_deref()),
None => (
executor_capability_discovery.as_deref(),
executor_capability_discovery_v2.as_deref(),
),
};
let selected_plugins = select_plugins(skill_discovery, skill_discovery_v2).await;
// V2 hooks match accepted installations from the session inventory. V1 hooks
// only need the explicit disabled IDs, already present in selected_plugins.
let session_plugins =
if turn_skill_discovery.is_some() && executor_capability_discovery_v2.is_some() {
Some(
select_plugins(
executor_capability_discovery.as_deref(),
executor_capability_discovery_v2.as_deref(),
)
.await,
)
} else {
None
};
let executor_hook_sources = crate::hook_runtime::prepare_executor_hook_sources(
&mcp,
session_plugins.as_ref().unwrap_or(&selected_plugins),
executor_capability_discovery.as_deref(),
executor_capability_discovery_v2.as_deref(),
);
let mut filesystem_access = HashMap::new();
for environment in environments.turn_environments() {
let environment_id = &environment.selection.environment_id;
@@ -4508,9 +4562,8 @@ impl Session {
.prepare_for_step(codex_extension_api::StepPreparationInput {
turn_id: &turn_context.sub_id,
ready_selected_capability_roots: &accessed_roots,
executor_capability_discovery: executor_capability_discovery.as_deref(),
executor_capability_discovery_v2: executor_capability_discovery_v2
.as_deref(),
executor_capability_discovery: skill_discovery,
executor_capability_discovery_v2: skill_discovery_v2,
selected_plugins: &selected_plugins,
session_store: &self.services.session_extension_data,
thread_store: &self.services.thread_extension_data,
@@ -4520,12 +4573,6 @@ impl Session {
.or_cancel(cancellation_token)
.await?;
}
let executor_hook_sources = crate::hook_runtime::prepare_executor_hook_sources(
&mcp,
&selected_plugins,
executor_capability_discovery.as_deref(),
executor_capability_discovery_v2.as_deref(),
);
extension_data.insert(selected_plugins);
let tool_router = turn::built_tools(
self.as_ref(),
codex-rs/core/src/session/tests.rs
+1
View File
@@ -10290,6 +10290,7 @@ async fn capability_discovery_uses_environment_permission_profile() {
/*resolved_roots*/ &[],
&turn_context.initial_environments,
/*v2_snapshots*/ None,
/*skill_turn*/ None,
)
.await;
let discovery = discovery.expect("restricted environment should trigger capability discovery");
codex-rs/core/src/state/service.rs
+15
View File
@@ -108,6 +108,21 @@ pub(crate) struct SessionServices {
}
impl SessionServices {
#[expect(
clippy::expect_used,
reason = "poisoned grants must not authorize discovery"
)]
pub(crate) fn granted_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
self.granted_permissions_by_environment_id
.lock()
.expect("session permission grants lock poisoned")
.get(environment_id)
.cloned()
}
#[expect(
clippy::expect_used,
reason = "poisoned grant state must not authorize further operations"
codex-rs/core/tests/common/exec_server.rs
+10 -2
View File
@@ -36,7 +36,10 @@ impl ExecServerProcess {
}
pub async fn start_with_version(version: &str) -> Result<Self> {
let codex_home = TempDir::new()?;
Self::start_with_version_and_home(version, TempDir::new()?).await
}
pub async fn start_with_version_and_home(version: &str, codex_home: TempDir) -> Result<Self> {
let package = codex_home.path().join("package");
let bin = package.join("bin");
std::fs::create_dir_all(&bin)?;
@@ -49,7 +52,12 @@ impl ExecServerProcess {
package.join("codex-package.json"),
serde_json::to_vec(&serde_json::json!({"version": version}))?,
)?;
Self::start_command(Command::new(executable), codex_home).await
let mut command = Command::new(executable);
// V2 must not discover skills installed in the developer's home directory.
command
.env("HOME", codex_home.path())
.env("USERPROFILE", codex_home.path());
Self::start_command(command, codex_home).await
}
async fn start_command(mut command: Command, codex_home: TempDir) -> Result<Self> {
codex-rs/core/tests/suite/executor_discovery_v2.rs
+139 -11
View File
@@ -1,4 +1,4 @@
//! Verifies V2 discovery, plugin acceptance across auth changes, and cache behavior.
//! Verifies executor discovery, plugin acceptance across auth changes, and cache behavior.
use anyhow::Context;
use anyhow::Result;
@@ -193,22 +193,24 @@ impl ContextContributor for DiscoveryRecorder {
}
}
struct Executor {
url: String,
inventory: Arc<Mutex<Option<Value>>>,
requests: Arc<Mutex<Vec<Value>>>,
pub(super) struct Executor {
pub(super) url: String,
pub(super) inventory: Arc<Mutex<Option<Value>>>,
pub(super) requests: Arc<Mutex<Vec<Value>>>,
_task: AbortOnDropHandle<Result<()>>,
}
#[derive(Clone, Copy)]
enum DiscoveryFailure {
pub(super) enum DiscoveryFailure {
None,
Server,
Disconnect,
#[cfg_attr(windows, expect(dead_code, reason = "executor hook tests run on Unix"))]
TurnGrant,
}
impl Executor {
async fn start(supports_v2: bool, failure: DiscoveryFailure) -> Result<Self> {
pub(super) async fn start(supports_v2: bool, failure: DiscoveryFailure) -> Result<Self> {
Self::start_with_version(supports_v2, failure, "0.164.0").await
}
@@ -265,23 +267,27 @@ impl Executor {
match failure {
// Exhaust the environment's one transport replay so the cache sees Err.
DiscoveryFailure::Disconnect if attempt < 2 => break,
DiscoveryFailure::TurnGrant if request["params"]["sandbox"].to_string().contains("turn-only-skills") => {
socket.send(Message::Text(json!({"id":request["id"],"error":{"code":-32004,"message":"turn discovery failed"}}).to_string().into())).await?;
continue;
}
DiscoveryFailure::Server if attempt == 0 => {
socket.send(Message::Text(json!({"id":request["id"],"error":{"code":-32004,"message":"fixture discovery failure"}}).to_string().into())).await?;
continue;
}
DiscoveryFailure::None | DiscoveryFailure::Server | DiscoveryFailure::Disconnect => {}
DiscoveryFailure::None | DiscoveryFailure::Server | DiscoveryFailure::Disconnect | DiscoveryFailure::TurnGrant => {}
}
}
let result = match request["method"].as_str() {
Some("initialize") => Some(json!({"sessionId":"acquisition-only", "environmentInfo": {
"shell":{"name":"bash","path":"/bin/bash"},
"executorVersion":version,
"capabilities":{"sandboxedFileStreaming":true,"capabilityDiscoveryV2":supports_v2}
"capabilities":{"sandboxedFileStreaming":true,"capabilityDiscoverySandbox":true,"capabilityDiscoveryV2":supports_v2}
}})),
Some("environment/info") => Some(json!({
"shell":{"name":"bash","path":"/bin/bash"},
"executorVersion":version,
"capabilities":{"sandboxedFileStreaming":true,"capabilityDiscoveryV2":supports_v2}
"capabilities":{"sandboxedFileStreaming":true,"capabilityDiscoverySandbox":true,"capabilityDiscoveryV2":supports_v2}
})),
Some("capabilityRoots/discoverV1") => Some(json!({
"roots": request["params"]["roots"].as_array().context("V1 roots")?
@@ -537,7 +543,9 @@ async fn v2_discovery_failures_retry_or_remain_cached_by_error_kind() -> Result<
// Two disconnected attempts, then one shared successful retry in the same cache.
3
}
DiscoveryFailure::None => unreachable!("the test only exercises failures"),
DiscoveryFailure::None | DiscoveryFailure::TurnGrant => {
unreachable!("the test only exercises startup failures")
}
};
assert_eq!(
executor
@@ -840,6 +848,126 @@ async fn check_acquisition(enabled: bool, version: &'static str) -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn v1_reuses_session_and_turn_discovery_across_model_steps() -> Result<()> {
skip_if_no_network!(Ok(()));
let executor = Executor::start(/*supports_v2*/ false, DiscoveryFailure::None).await?;
let model = responses::start_mock_server().await;
let test = test_codex()
.with_exec_server_url(executor.url.clone())
.with_config(|config| {
config.update_plan_enabled = true;
config
.features
.enable(Feature::RequestPermissionsTool)
.unwrap();
config
.features
.disable(Feature::ExecutorCapabilityDiscoveryV2)
.unwrap();
config.permissions.approval_policy = codex_core::config::Constrained::allow_any(
codex_protocol::protocol::AskForApproval::OnRequest,
);
config
.permissions
.set_permission_profile(
codex_protocol::models::PermissionProfile::from_runtime_permissions(
&codex_protocol::permissions::FileSystemSandboxPolicy::restricted(
Vec::new(),
),
codex_protocol::permissions::NetworkSandboxPolicy::Restricted,
),
)
.unwrap();
})
.build_with_auto_env(&model)
.await?;
let selection = test.codex.environment_selections().await.remove(0);
let mut config =
core_test_support::test_codex::environment_config_for_selection(&test.config, &selection);
config.selected_capability_roots = vec![codex_protocol::capabilities::SelectedCapabilityRoot {
id: "legacy-root".to_string(),
location: codex_protocol::capabilities::CapabilityRootLocation::Environment {
environment_id: selection.environment_id.clone(),
path: selection.cwd.clone(),
},
}];
test.codex.environment_ready(&selection, config).await?;
let permissions = codex_protocol::request_permissions::RequestPermissionProfile {
file_system: Some(
codex_protocol::models::FileSystemPermissions::from_read_write_path_uris(
Some(vec![selection.cwd.join("approved")?]),
/*write*/ None,
),
),
..Default::default()
};
let mut sequence = vec![responses::sse(vec![
responses::ev_function_call(
"grant",
"request_permissions",
&json!({
"permissions": permissions,
})
.to_string(),
),
responses::ev_completed("grant"),
])];
for id in ["first-step", "second-step"] {
sequence.push(responses::sse(vec![
responses::ev_function_call(
id,
"update_plan",
r#"{"plan":[{"step":"Inspect skills","status":"completed"}]}"#,
),
responses::ev_completed(id),
]));
}
sequence.push(responses::sse(vec![responses::ev_completed("done")]));
let responses = responses::mount_sse_sequence(&model, sequence).await;
test.codex
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
text: "Inspect the approved skills across two steps.".into(),
text_elements: Vec::new(),
}]))
.await?;
let request = core_test_support::wait_for_event_match(&test.codex, |event| match event {
EventMsg::RequestPermissions(request) => Some(request.clone()),
_ => None,
})
.await;
test.codex
.submit(Op::RequestPermissionsResponse {
id: request.call_id,
response: codex_protocol::request_permissions::RequestPermissionsResponse {
permissions,
scope: codex_protocol::request_permissions::PermissionGrantScope::Turn,
strict_auto_review: false,
},
})
.await?;
core_test_support::wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
assert_eq!(responses.requests().len(), 4);
let requests = executor.requests.lock().await;
let discovery = requests
.iter()
.filter(|request| request["method"] == "capabilityRoots/discoverV1")
.collect::<Vec<_>>();
assert_eq!(
discovery.len(),
2,
"unchanged steps reuse both discovery scopes"
);
assert_ne!(
discovery[0]["params"]["roots"][0]["sandbox"],
discovery[1]["params"]["roots"][0]["sandbox"]
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn v2_discovery_cache_reuses_matching_requests_and_isolates_execution_contexts() -> Result<()>
{
codex-rs/core/tests/suite/hooks_executor.rs
+137 -1
View File
@@ -711,6 +711,138 @@ async fn thread_plugin_selection_disables_executor_hooks_without_disabling_their
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn session_hooks_survive_turn_skill_discovery_failure() -> Result<()> {
skip_if_no_network!(Ok(()));
let executor = super::executor_discovery_v2::Executor::start(
/*supports_v2*/ true,
super::executor_discovery_v2::DiscoveryFailure::TurnGrant,
)
.await?;
*executor.inventory.lock().await = Some(json!({
"plugins": [{
"id": "computer-use@openai-bundled", "displayName": "computer-use",
"remotePluginId": null, "version": "1", "root": "file:///computer-use",
"files": {
"manifest": {"path": "file:///computer-use/.codex-plugin/plugin.json",
"contents": computer_use_hook_manifest().to_string()},
"mcpConfig": null, "appsConfig": null,
}, "skills": [],
}],
"skills": [], "warnings": [],
}));
let mut extensions = codex_extension_api::ExtensionRegistryBuilder::new();
codex_mcp_extension::install_plugins(
&mut extensions,
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
);
let fixture = executor_plugin_hook_fixture(
test_codex()
.with_exec_server_url(executor.url.clone())
.with_extensions(Arc::new(extensions.build()))
.with_config(|config| {
for feature in [
Feature::ExecutorCapabilityDiscoveryV2,
Feature::Plugins,
Feature::RequestPermissionsTool,
] {
config.features.enable(feature).unwrap();
}
config.config_layer_stack = config
.config_layer_stack
.with_user_config(
&config.codex_home.join("config.toml"),
toml::from_str("[plugins.\"computer-use@openai-bundled\"]\nenabled = true")
.unwrap(),
)
.unwrap();
config.permissions.approval_policy = codex_core::config::Constrained::allow_any(
codex_protocol::protocol::AskForApproval::OnRequest,
);
config
.permissions
.set_permission_profile(
codex_protocol::models::PermissionProfile::from_runtime_permissions(
&codex_protocol::permissions::FileSystemSandboxPolicy::restricted(
Vec::new(),
),
codex_protocol::permissions::NetworkSandboxPolicy::Restricted,
),
)
.unwrap();
}),
&[],
vec![
sse_response(sse(vec![
ev_function_call(
"grant",
"request_permissions",
r#"{"permissions":{"file_system":{"read":["turn-only-skills"]}}}"#,
),
ev_completed("grant"),
])),
completed_turn_response("done"),
completed_turn_response("disabled"),
],
)
.await?;
fixture.attach().await?;
fixture
.test
.codex
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
text: "Grant skill access, then finish.".into(),
text_elements: Vec::new(),
}]))
.await?;
let request = wait_for_event_match(&fixture.test.codex, |event| match event {
EventMsg::RequestPermissions(request) => Some(request.clone()),
_ => None,
})
.await;
fixture
.test
.codex
.submit(Op::RequestPermissionsResponse {
id: request.call_id,
response: codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: request.permissions,
scope: codex_protocol::request_permissions::PermissionGrantScope::Turn,
strict_auto_review: false,
},
})
.await?;
wait_for_event(&fixture.test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
fixture.wait_for_hook_call().await?;
assert_eq!(fixture.calls().await?.len(), 1);
assert!(
executor.requests.lock().await.iter().any(|request| {
request["method"] == "capabilities/discoverV2"
&& request["params"]["sandbox"]
.to_string()
.contains("turn-only-skills")
}),
"the turn inventory failed while the session inventory stayed healthy"
);
submit_thread_settings(
&fixture.test.codex,
ThreadSettingsOverrides {
disabled_plugin_ids: Some(vec!["computer-use@openai-bundled".into()]),
..Default::default()
},
)
.await?;
fixture
.test
.submit_text_turn("Finish with the plugin disabled.")
.await?;
assert_eq!(fixture.calls().await?.len(), 1);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn executor_stop_hook_uses_plugin_policy_refreshed_during_turn() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -1454,7 +1586,11 @@ async fn executor_plugin_hook_fixture(
.expect("disable ordinary hooks");
let node_repl: McpServerConfig = serde_json::from_value(json!({
"url": node_repl_url,
"environment_id": remote_aware_environment_id(),
"environment_id": if config.features.enabled(Feature::ExecutorCapabilityDiscoveryV2) {
codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string()
} else {
remote_aware_environment_id()
},
}))
.expect("valid Node REPL MCP server configuration");
config
codex-rs/core/tests/suite/skills_extension.rs
+3
View File
@@ -4167,3 +4167,6 @@ async fn production_turn_fairly_shortens_extension_catalog_descriptions() -> Res
Ok(())
}
#[path = "skills_extension/discovery_grants_tests.rs"]
mod discovery_grants_tests;
codex-rs/core/tests/suite/skills_extension/discovery_grants_tests.rs
+519
View File
@@ -0,0 +1,519 @@
#![cfg(test)]
//! Exercises skill and plugin discovery before approval, after approval, and next turn.
use super::*;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
use core_test_support::exec_server::ExecServerProcess;
use core_test_support::test_codex::TestEnv;
use pretty_assertions::assert_eq;
use test_case::test_case;
#[derive(Clone, Copy)]
enum Discovery {
ExecutorV1,
DisabledExecutorV1Plugin,
DisabledExecutorV1UnreadablePlugin,
DisabledExecutorV1UnreadablePluginDirectory,
DisabledExecutorV1UnreadableAncestorPlugin,
DisabledExecutorV1UnreadableAncestorPluginDirectory,
StandaloneExecutorV1WithDisabledId,
ExecutorV2,
}
#[test_case(Discovery::ExecutorV1, PermissionGrantScope::Session; "executor v1 session grant")]
#[test_case(Discovery::ExecutorV1, PermissionGrantScope::Turn; "executor v1 turn grant")]
#[test_case(Discovery::ExecutorV2, PermissionGrantScope::Session; "executor v2 session grant")]
#[test_case(Discovery::ExecutorV2, PermissionGrantScope::Turn; "executor v2 turn grant")]
#[test_case(Discovery::DisabledExecutorV1Plugin, PermissionGrantScope::Turn; "turn grant respects plugin disablement")]
#[test_case(Discovery::DisabledExecutorV1UnreadablePlugin, PermissionGrantScope::Turn; "turn grant cannot bypass unreadable disabled plugin manifest")]
#[test_case(Discovery::DisabledExecutorV1UnreadablePluginDirectory, PermissionGrantScope::Turn; "turn grant cannot bypass unreadable disabled plugin directory")]
#[test_case(Discovery::DisabledExecutorV1UnreadableAncestorPlugin, PermissionGrantScope::Turn; "turn grant cannot bypass unreadable disabled ancestor plugin manifest")]
#[test_case(Discovery::DisabledExecutorV1UnreadableAncestorPluginDirectory, PermissionGrantScope::Turn; "turn grant cannot bypass unreadable disabled ancestor plugin directory")]
#[test_case(Discovery::StandaloneExecutorV1WithDisabledId, PermissionGrantScope::Turn; "explicit plugin deny preserves a standalone root")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn executor_discovery_uses_current_environment_grants(
discovery: Discovery,
scope: PermissionGrantScope,
) -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_target_windows!(
Ok(()),
"restricted reads require the elevated Windows sandbox backend, unavailable in this fixture"
);
let v2 = matches!(discovery, Discovery::ExecutorV2);
let plugin_enabled = !matches!(
discovery,
Discovery::DisabledExecutorV1Plugin
| Discovery::DisabledExecutorV1UnreadablePlugin
| Discovery::DisabledExecutorV1UnreadablePluginDirectory
| Discovery::DisabledExecutorV1UnreadableAncestorPlugin
| Discovery::DisabledExecutorV1UnreadableAncestorPluginDirectory
);
let plugin_mcp_server = "approved_plugin_mcp";
let environment = if v2 {
None
} else {
Some(core_test_support::test_codex::test_env().await?)
};
let mut extensions = ExtensionRegistryBuilder::new();
if let Some(environment) = &environment {
#[cfg(target_os = "linux")]
let sandbox_exe = Some(core_test_support::find_codex_linux_sandbox_exe()?);
#[cfg(not(target_os = "linux"))]
let sandbox_exe = None;
let runtime_paths = codex_exec_server::ExecServerRuntimeOptions::new(
std::env::current_exe()?,
sandbox_exe,
)?;
codex_mcp_extension::install_plugins(
&mut extensions,
Arc::new(
codex_exec_server::EnvironmentManager::create_for_tests(
environment.exec_server_url().map(str::to_owned),
Some(runtime_paths),
)
.await,
),
);
}
let providers = SkillProviders::new().with_executor_provider(Arc::new(
ExecutorSkillProvider::new_with_restriction_product(/*restriction_product*/ None),
));
install_with_providers(&mut extensions, providers, |config: &Config| {
SkillsExtensionConfig {
include_instructions: config.include_skill_instructions,
max_context_tokens: config.skill_max_context_tokens,
catalog_deltas_enabled: config.features.enabled(Feature::SkillCatalogDeltas),
omit_descriptions: false,
bundled_skills_enabled: false,
cloud_skill_enabled: false,
shadow_selection_enabled: false,
}
});
let mut builder = test_codex()
.with_extensions(Arc::new(extensions.build()))
.with_config(move |config| {
configure_catalog_test(config);
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("permission tool");
config
.features
.set_enabled(Feature::ExecutorCapabilityDiscoveryV2, v2)
.expect("executor discovery version");
config
.features
.set_enabled(Feature::Plugins, !v2)
.expect("selected plugins in V1");
config
.features
.enable(Feature::SkipHostSkillDiscovery)
.expect("executor-only discovery");
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config
.permissions
.set_permission_profile(PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(Vec::new()),
NetworkSandboxPolicy::Restricted,
))
.expect("no initial filesystem access");
});
let server = responses::start_mock_server().await;
// V2 requires a remote executor with the final protocol version.
let executor;
let (test, root) = if v2 {
skip_if_remote!(Ok(()), "this case starts its own native exec-server");
let home = tempfile::tempdir()?;
let root = PathUri::from_host_native_path(home.path().canonicalize()?.join("skills"))?;
// V2 scans installed roots; create this root before the executor prewarms its locations.
std::fs::create_dir(home.path().join("skills"))?;
executor = ExecServerProcess::start_with_version_and_home("0.164.0", home).await?;
let environment =
TestEnv::local_with_exec_server_url(Some(executor.websocket_url.clone())).await?;
(
builder.build_with_environment(&server, environment).await?,
root,
)
} else {
let test = builder
.build_with_environment(&server, environment.expect("V1 test environment"))
.await?;
let root = test
.executor_environment()
.selection()
.cwd
.join(".agents/skills")?;
(test, root)
};
let selection = test.executor_environment().selection().clone();
let plugin_root = if matches!(
discovery,
Discovery::DisabledExecutorV1UnreadableAncestorPlugin
| Discovery::DisabledExecutorV1UnreadableAncestorPluginDirectory
) {
root.parent()
.expect("plugin owns selected skill subdirectory")
} else {
root.clone()
};
let skill = root.join("approved-skill")?;
test.fs()
.create_directory(
&skill,
CreateDirectoryOptions {
recursive: true,
follow_symlinks: true,
},
/*sandbox*/ None,
)
.await?;
test.fs().write_file(
&skill.join("SKILL.md")?,
b"---\nname: approved-skill\ndescription: Approved repository instructions.\n---\n\nApproved skill body.".to_vec(),
Default::default(),
/*sandbox*/ None,
).await?;
if !v2 && !matches!(discovery, Discovery::StandaloneExecutorV1WithDisabledId) {
let directory = if matches!(
discovery,
Discovery::DisabledExecutorV1UnreadablePluginDirectory
| Discovery::DisabledExecutorV1UnreadableAncestorPluginDirectory
) {
plugin_root
.parent()
.expect("root parent")
.join("denied-plugin")?
} else {
plugin_root.join(".codex-plugin")?
};
test.fs()
.create_directory(
&directory,
CreateDirectoryOptions {
recursive: true,
follow_symlinks: true,
},
/*sandbox*/ None,
)
.await?;
let manifest = plugin_root.join(".codex-plugin/plugin.json")?;
let target = if matches!(
discovery,
Discovery::DisabledExecutorV1UnreadablePlugin
| Discovery::DisabledExecutorV1UnreadableAncestorPlugin
) {
plugin_root
.parent()
.expect("root parent")
.join("denied-plugin.json")?
} else {
directory.join("plugin.json")?
};
test.fs()
.write_file(
&target,
br#"{"name":"approved-plugin"}"#.to_vec(),
Default::default(),
/*sandbox*/ None,
)
.await?;
let (target, link) = if matches!(
discovery,
Discovery::DisabledExecutorV1UnreadablePluginDirectory
| Discovery::DisabledExecutorV1UnreadableAncestorPluginDirectory
) {
(directory, plugin_root.join(".codex-plugin")?)
} else {
(target, manifest)
};
if target != link {
let process = test
.executor_environment()
.environment()
.get_exec_backend()
.start(ExecParams {
process_id: ProcessId::from("link-denied-plugin-manifest"),
metadata: None,
argv: vec![
"/bin/ln".to_string(),
"-s".to_string(),
target.inferred_native_path_string(),
link.inferred_native_path_string(),
],
cwd: selection.cwd.clone(),
env_policy: None,
shell_snapshot: None,
env: Default::default(),
tty: false,
pipe_stdin: false,
arg0: None,
sandbox: None,
enforce_managed_network: false,
managed_network: None,
network_proxy: None,
})
.await?
.process;
let mut events = process.subscribe_events();
loop {
match events.recv().await? {
ExecProcessEvent::Exited { exit_code, .. } => {
assert_eq!(exit_code, 0);
break;
}
ExecProcessEvent::Output(_) => {}
event @ (ExecProcessEvent::Closed { .. } | ExecProcessEvent::Failed(_)) => {
panic!("manifest symlink process did not exit: {event:?}")
}
}
}
}
let command = crate::suite::rmcp_client::remote_aware_stdio_server_bin()?;
test.fs()
.write_file(
&root.join(".mcp.json")?,
serde_json::to_vec(&json!({
"mcpServers": {(plugin_mcp_server): {"command": command}},
}))?,
Default::default(),
/*sandbox*/ None,
)
.await?;
}
let package = format!(
"skill://{}/{}",
match discovery {
Discovery::ExecutorV1
| Discovery::DisabledExecutorV1Plugin
| Discovery::DisabledExecutorV1UnreadablePlugin
| Discovery::DisabledExecutorV1UnreadablePluginDirectory
| Discovery::DisabledExecutorV1UnreadableAncestorPlugin
| Discovery::DisabledExecutorV1UnreadableAncestorPluginDirectory
| Discovery::StandaloneExecutorV1WithDisabledId => "approved-root",
Discovery::ExecutorV2 => "codex-home-skills",
},
skill
.inferred_native_path_string()
.replace('\\', "/")
.trim_start_matches('/')
);
let read_skill = |call_id| {
responses::ev_function_call_with_namespace(
call_id,
"skills",
"read",
&json!({"package": package}).to_string(),
)
};
let mut thread_extension_init = ExtensionDataInit::new();
if !v2 {
thread_extension_init.insert(vec![SelectedCapabilityRoot {
id: "approved-root".to_string(),
location: CapabilityRootLocation::Environment {
environment_id: selection.environment_id.clone(),
path: root.clone(),
},
}]);
}
let thread = test
.thread_manager
.start_thread(StartThreadOptions {
environments: Some(vec![selection.into_request()]),
disabled_plugin_ids: (!plugin_enabled
|| matches!(discovery, Discovery::StandaloneExecutorV1WithDisabledId))
.then(|| vec!["approved-root".to_string()]),
thread_extension_init,
..StartThreadOptions::new(test.config.clone())
})
.await?
.thread;
let plugin_is_published = async || -> Result<bool> {
let (config, status) = thread
.mcp_server_status_snapshot(
plugin_mcp_server,
codex_mcp::McpSnapshotDetail::ToolsAndAuthOnly,
/*expected_registration*/ None,
)
.await?;
let published = config
.mcp_server_catalog
.server(plugin_mcp_server)
.is_some();
let has_echo = status
.tools_by_server
.get(plugin_mcp_server)
.is_some_and(|tools| tools.contains_key("echo"));
assert_eq!(has_echo, published);
Ok(published)
};
let permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_path_uris(
Some(vec![
test.fs()
.canonicalize(&plugin_root, /*sandbox*/ None)
.await?,
]),
/*write*/ None,
)),
..Default::default()
};
let mut sequence = vec![
sse(vec![
ev_response_created("before-approval"),
read_skill("read-before-approval"),
ev_completed("before-approval"),
]),
sse(vec![
ev_response_created("grant"),
responses::ev_function_call(
"grant-skills",
"request_permissions",
&json!({
"reason": "Read repository skills", "permissions": permissions,
})
.to_string(),
),
ev_completed("grant"),
]),
];
for (response_id, call_id) in [
("granted", "read-after-approval"),
("next-turn", "read-next-turn"),
] {
let mut events = vec![ev_response_created(response_id), read_skill(call_id)];
if response_id == "granted" && !v2 {
events.push(responses::ev_function_call(
"request-sibling",
"request_permissions",
&json!({
"reason": "Read the sibling file",
"permissions": {"file_system": FileSystemPermissions::from_read_write_path_uris(
Some(vec![root.parent().expect("skills parent").join("private")?]), /*write*/ None,
)},
})
.to_string(),
));
}
events.push(ev_completed(response_id));
sequence.push(sse(events));
sequence.push(sse(vec![ev_response_created("done"), ev_completed("done")]));
}
let responses = responses::mount_sse_sequence(&server, sequence).await;
thread
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
text: "Read the selected skill, requesting permission if needed.".into(),
text_elements: Vec::new(),
}]))
.await?;
let event = wait_for_event(&thread, |event| {
matches!(
event,
EventMsg::RequestPermissions(_) | EventMsg::TurnComplete(_)
)
})
.await;
let EventMsg::RequestPermissions(request) = event else {
panic!("expected permission request: {event:?}");
};
assert_eq!(request.permissions, permissions);
if !v2 {
assert!(!plugin_is_published().await?);
}
thread
.submit(Op::RequestPermissionsResponse {
id: request.call_id,
response: RequestPermissionsResponse {
permissions,
scope,
strict_auto_review: false,
},
})
.await?;
if !v2 {
// The second permission request keeps the turn open while we inspect the live grant.
let event = wait_for_event(&thread, |event| {
matches!(
event,
EventMsg::RequestPermissions(_) | EventMsg::TurnComplete(_)
)
})
.await;
let EventMsg::RequestPermissions(request) = event else {
panic!("expected the sibling permission request: {event:?}");
};
assert_eq!(request.call_id, "request-sibling");
assert_eq!(
plugin_is_published().await?,
scope == PermissionGrantScope::Session
);
thread
.submit(Op::RequestPermissionsResponse {
id: request.call_id,
response: RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
})
.await?;
}
wait_for_event(&thread, |event| matches!(event, EventMsg::TurnComplete(_))).await;
if !v2 {
assert_eq!(
plugin_is_published().await?,
scope == PermissionGrantScope::Session
);
}
thread
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
text: "Read the selected skill again.".into(),
text_elements: Vec::new(),
}]))
.await?;
wait_for_event(&thread, |event| matches!(event, EventMsg::TurnComplete(_))).await;
let requests = responses.requests();
assert_eq!(requests.len(), 6);
let visible = [0, 2, 4]
.into_iter()
.map(|index| {
requests[index]
.message_input_texts("developer")
.iter()
.rev()
.find(|text| text.contains(SKILLS_INSTRUCTIONS_OPEN_TAG))
.is_some_and(|text| text.contains("approved-skill"))
})
.collect::<Vec<_>>();
assert_eq!(
visible,
vec![
false,
plugin_enabled,
plugin_enabled && scope == PermissionGrantScope::Session
]
);
let readable = [
(1, "read-before-approval"),
(3, "read-after-approval"),
(5, "read-next-turn"),
]
.into_iter()
.map(|(index, call)| {
requests[index]
.function_call_output_text(call)
.expect("skill read result")
.contains("Approved skill body.")
})
.collect::<Vec<_>>();
assert_eq!(
readable,
vec![
false,
plugin_enabled,
plugin_enabled && scope == PermissionGrantScope::Session
]
);
Ok(())
}
codex-rs/ext/extension-api/src/contributors/mcp.rs
+15
View File
@@ -18,6 +18,8 @@ use crate::ExtensionFuture;
pub struct McpServerContributionContext<'a, C> {
/// Host configuration visible during MCP resolution.
config: &'a C,
/// Explicit thread denials, including V1 roots whose manifests cannot be read.
disabled_plugin_ids: &'a [String],
/// Whether pending auth differs from published auth; invalidate catalogs before projection.
auth_changed: bool,
/// Extension-owned data for the active thread, when resolution is thread-scoped.
@@ -51,6 +53,7 @@ impl<'a, C> McpServerContributionContext<'a, C> {
pub fn global(config: &'a C) -> Self {
Self {
config,
disabled_plugin_ids: &[],
auth_changed: false,
thread_store: None,
thread_init: None,
@@ -75,6 +78,7 @@ impl<'a, C> McpServerContributionContext<'a, C> {
) -> Self {
Self {
config,
disabled_plugin_ids: &[],
auth_changed: false,
thread_store: Some(thread_store),
thread_init: Some(thread_init),
@@ -87,6 +91,17 @@ impl<'a, C> McpServerContributionContext<'a, C> {
}
}
/// Applies thread denials before resolving ownership of unreadable plugin manifests.
pub fn with_disabled_plugin_ids(mut self, ids: &'a [String]) -> Self {
self.disabled_plugin_ids = ids;
self
}
/// Plugin identities explicitly disabled for the active thread.
pub fn disabled_plugin_ids(&self) -> &'a [String] {
self.disabled_plugin_ids
}
/// Attaches the same environment snapshot used to project MCP authority.
///
/// Preserve pending and failed selections: an unavailable primary executor
codex-rs/ext/mcp/src/plugin.rs
+7 -2
View File
@@ -343,7 +343,10 @@ impl McpServerContributor<Config> for PluginContributor {
&root.selected_root.location;
// Ownership survives capability parse failures; an unresolved root cannot
// bypass a denying policy.
if !plugins_config.allows_plugin(&root.selected_root.id)
if (!plugins_config.allows_plugin(&root.selected_root.id)
|| context
.disabled_plugin_ids()
.contains(&root.selected_root.id))
&& discovery.error.is_none()
&& !matches!(
self.ownership_for_root(
@@ -389,7 +392,9 @@ impl McpServerContributor<Config> for PluginContributor {
}
} else {
for selected_root in selected_roots {
if !plugins_config.allows_plugin(&selected_root.id) {
if !plugins_config.allows_plugin(&selected_root.id)
|| context.disabled_plugin_ids().contains(&selected_root.id)
{
if !matches!(
self.ownership_for_root(
&state,
codex-rs/ext/mcp/tests/plugin_mcp.rs
+3 -3
View File
@@ -303,7 +303,7 @@ async fn selected_root_ownership_retries_errors_and_caches_success() -> TestResu
for (phase, enabled_root, expected) in [
(0, None, vec![("standalone", vec![])]),
(1, None, vec![]),
(1, None, vec![("standalone", vec![])]),
(2, Some("owned-subfolder"), vec![("selected-root", vec![])]),
(
3,
@@ -350,8 +350,8 @@ async fn selected_root_ownership_retries_errors_and_caches_success() -> TestResu
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.build()
.await?;
// A warning requires metadata probing, which the test filesystem cannot sandbox.
// Complete discovery retries ownership under the same sandbox, root, and policy.
// Neither incomplete nor legacy complete discovery proves that a marker is absent.
// The scoped metadata probe fails here; later readable roots must retry ownership.
let discovery = (phase < 2).then(|| {
ExecutorCapabilityDiscoverySnapshot::new(
std::slice::from_ref(standalone),