agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Revert "fix: harden plugin feature gating" (#15102)

Browse Source 
Reverts openai/codex#15020

I messed up the commit in my PR and accidentally merged changes that
were still under review.
This commit is contained in:
xl-openai
2026-03-18 15:19:29 -07:00
committed by GitHub
parent e5de13644d
commit 86982ca1f9
40 changed files with 52 additions and 926 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+6 -14
View File
@@ -2385,12 +2385,11 @@ impl Session {
&per_turn_config,
)
.await;
let skills_outcome = Arc::new(crate::skills::filter_skill_load_outcome_for_session_source(
let skills_outcome = Arc::new(
self.services
.skills_manager
.skills_for_config(&per_turn_config),
&session_configuration.session_source,
));
);
let mut turn_context: TurnContext = Self::make_turn_context(
Some(Arc::clone(&self.services.auth_manager)),
&self.services.session_telemetry,
@@ -4774,24 +4773,17 @@ mod handlers {
cwds: Vec<PathBuf>,
force_reload: bool,
) {
let (cwds, session_source) = if cwds.is_empty() {
let cwds = if cwds.is_empty() {
let state = sess.state.lock().await;
(
vec![state.session_configuration.cwd.clone()],
state.session_configuration.session_source.clone(),
)
vec![state.session_configuration.cwd.clone()]
} else {
let state = sess.state.lock().await;
(cwds, state.session_configuration.session_source.clone())
cwds
};
let skills_manager = &sess.services.skills_manager;
let mut skills = Vec::new();
for cwd in cwds {
let outcome = crate::skills::filter_skill_load_outcome_for_session_source(
skills_manager.skills_for_cwd(&cwd, force_reload).await,
&session_source,
);
let outcome = skills_manager.skills_for_cwd(&cwd, force_reload).await;
let errors = super::errors_to_info(&outcome.errors);
let skills_metadata = super::skills_to_info(&outcome.skills, &outcome.disabled_paths);
skills.push(SkillsListEntry {
codex-rs/core/src/plugins/manager.rs
+5 -35
View File
@@ -44,7 +44,6 @@ use crate::skills::loader::SkillRoot;
use crate::skills::loader::load_skills_from_roots;
use codex_app_server_protocol::ConfigValueWriteParams;
use codex_app_server_protocol::MergeStrategy;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
@@ -939,11 +938,7 @@ impl PluginsManager {
})
}
pub fn maybe_start_curated_repo_sync_for_config(
self: &Arc<Self>,
config: &Config,
session_source: &SessionSource,
) {
pub fn maybe_start_curated_repo_sync_for_config(self: &Arc<Self>, config: &Config) {
if plugins_feature_enabled_from_stack(&config.config_layer_stack) {
let mut configured_curated_plugin_ids =
configured_plugins_from_stack(&config.config_layer_stack)
@@ -966,15 +961,11 @@ impl PluginsManager {
})
.collect::<Vec<_>>();
configured_curated_plugin_ids.sort_unstable_by_key(super::store::PluginId::as_key);
self.start_curated_repo_sync(configured_curated_plugin_ids, session_source.clone());
self.start_curated_repo_sync(configured_curated_plugin_ids);
}
}
fn start_curated_repo_sync(
self: &Arc<Self>,
configured_curated_plugin_ids: Vec<PluginId>,
session_source: SessionSource,
) {
fn start_curated_repo_sync(self: &Arc<Self>, configured_curated_plugin_ids: Vec<PluginId>) {
if CURATED_REPO_SYNC_STARTED.swap(true, Ordering::SeqCst) {
return;
}
@@ -989,7 +980,6 @@ impl PluginsManager {
codex_home.as_path(),
&curated_plugin_version,
&configured_curated_plugin_ids,
&session_source,
) {
Ok(cache_refreshed) => {
if cache_refreshed {
@@ -1215,7 +1205,6 @@ fn refresh_curated_plugin_cache(
codex_home: &Path,
plugin_version: &str,
configured_curated_plugin_ids: &[PluginId],
session_source: &SessionSource,
) -> Result<bool, String> {
let store = PluginStore::new(codex_home.to_path_buf());
let curated_marketplace_path = AbsolutePathBuf::try_from(
@@ -1226,12 +1215,9 @@ fn refresh_curated_plugin_cache(
.map_err(|err| format!("failed to load curated marketplace for cache refresh: {err}"))?;
let mut plugin_sources = HashMap::<String, AbsolutePathBuf>::new();
let mut product_restricted_plugin_names = HashSet::<String>::new();
for plugin in curated_marketplace.plugins {
let plugin_name = plugin.name;
if plugin_sources.contains_key(&plugin_name)
|| product_restricted_plugin_names.contains(&plugin_name)
{
if plugin_sources.contains_key(&plugin_name) {
warn!(
plugin = plugin_name,
marketplace = OPENAI_CURATED_MARKETPLACE_NAME,
@@ -1242,32 +1228,16 @@ fn refresh_curated_plugin_cache(
let source_path = match plugin.source {
MarketplacePluginSource::Local { path } => path,
};
if session_source.matches_product_restriction(&plugin.policy.products) {
plugin_sources.insert(plugin_name, source_path);
} else {
product_restricted_plugin_names.insert(plugin_name);
}
plugin_sources.insert(plugin_name, source_path);
}
let mut cache_refreshed = false;
for plugin_id in configured_curated_plugin_ids {
// Curated plugin cache entries are intentionally sticky across session source changes.
// Product restrictions gate refresh for this source, but do not retroactively evict an
// already-active cache entry from a shared CODEX_HOME.
if store.active_plugin_version(plugin_id).as_deref() == Some(plugin_version) {
continue;
}
let Some(source_path) = plugin_sources.get(&plugin_id.plugin_name).cloned() else {
if product_restricted_plugin_names.contains(&plugin_id.plugin_name) {
info!(
plugin = plugin_id.plugin_name,
marketplace = OPENAI_CURATED_MARKETPLACE_NAME,
session_source = %session_source,
"skipping curated plugin cache refresh for product-restricted plugin"
);
continue;
}
warn!(
plugin = plugin_id.plugin_name,
marketplace = OPENAI_CURATED_MARKETPLACE_NAME,
codex-rs/core/src/plugins/manager_tests.rs
+6 -80
View File
@@ -9,12 +9,10 @@ use crate::config_loader::ConfigRequirements;
use crate::config_loader::ConfigRequirementsToml;
use crate::plugins::MarketplacePluginInstallPolicy;
use crate::plugins::test_support::TEST_CURATED_PLUGIN_SHA;
use crate::plugins::test_support::write_curated_plugin;
use crate::plugins::test_support::write_curated_plugin_sha_with as write_curated_plugin_sha;
use crate::plugins::test_support::write_file;
use crate::plugins::test_support::write_openai_curated_marketplace;
use codex_app_server_protocol::ConfigLayerSource;
use codex_protocol::protocol::SessionSource;
use pretty_assertions::assert_eq;
use std::fs;
use tempfile::TempDir;
@@ -1034,12 +1032,6 @@ async fn list_marketplaces_includes_curated_repo_marketplace() {
r#"{"name":"linear"}"#,
)
.unwrap();
write_file(
&tmp.path().join(CONFIG_TOML_FILE),
r#"[features]
plugins = true
"#,
);
let config = load_config(tmp.path(), tmp.path()).await;
let marketplaces = PluginsManager::new(tmp.path().to_path_buf())
@@ -1635,13 +1627,8 @@ fn refresh_curated_plugin_cache_replaces_existing_local_version_with_sha() {
);
assert!(
refresh_curated_plugin_cache(
tmp.path(),
TEST_CURATED_PLUGIN_SHA,
&[plugin_id],
&SessionSource::Cli,
)
.expect("cache refresh should succeed")
refresh_curated_plugin_cache(tmp.path(), TEST_CURATED_PLUGIN_SHA, &[plugin_id])
.expect("cache refresh should succeed")
);
assert!(
@@ -1671,13 +1658,8 @@ fn refresh_curated_plugin_cache_reinstalls_missing_configured_plugin_with_curren
.unwrap();
assert!(
refresh_curated_plugin_cache(
tmp.path(),
TEST_CURATED_PLUGIN_SHA,
&[plugin_id],
&SessionSource::Cli,
)
.expect("cache refresh should recreate missing configured plugin")
refresh_curated_plugin_cache(tmp.path(), TEST_CURATED_PLUGIN_SHA, &[plugin_id])
.expect("cache refresh should recreate missing configured plugin")
);
assert!(
@@ -1706,64 +1688,8 @@ fn refresh_curated_plugin_cache_returns_false_when_configured_plugins_are_curren
);
assert!(
!refresh_curated_plugin_cache(
tmp.path(),
TEST_CURATED_PLUGIN_SHA,
&[plugin_id],
&SessionSource::Cli,
)
.expect("cache refresh should be a no-op when configured plugins are current")
);
}
#[test]
fn refresh_curated_plugin_cache_skips_product_restricted_plugins_for_session_source() {
let tmp = tempfile::tempdir().unwrap();
let curated_root = curated_plugins_repo_path(tmp.path());
write_file(
&curated_root.join(".agents/plugins/marketplace.json"),
&format!(
r#"{{
"name": "{OPENAI_CURATED_MARKETPLACE_NAME}",
"plugins": [
{{
"name": "chatgpt-plugin",
"source": {{
"source": "local",
"path": "./plugins/chatgpt-plugin"
}},
"policy": {{
"products": ["CHATGPT"]
}}
}}
]
}}"#
),
);
write_curated_plugin(&curated_root, "chatgpt-plugin");
write_curated_plugin_sha(tmp.path(), TEST_CURATED_PLUGIN_SHA);
let plugin_id = PluginId::new(
"chatgpt-plugin".to_string(),
OPENAI_CURATED_MARKETPLACE_NAME.to_string(),
)
.unwrap();
assert!(
!refresh_curated_plugin_cache(
tmp.path(),
TEST_CURATED_PLUGIN_SHA,
&[plugin_id],
&SessionSource::Cli,
)
.expect("cache refresh should skip disallowed product plugin")
);
assert!(
!tmp.path()
.join(format!(
"plugins/cache/openai-curated/chatgpt-plugin/{TEST_CURATED_PLUGIN_SHA}"
))
.exists()
!refresh_curated_plugin_cache(tmp.path(), TEST_CURATED_PLUGIN_SHA, &[plugin_id])
.expect("cache refresh should be a no-op when configured plugins are current")
);
}
codex-rs/core/src/plugins/marketplace.rs
+2
View File
@@ -54,6 +54,8 @@ pub enum MarketplacePluginSource {
pub struct MarketplacePluginPolicy {
pub installation: MarketplacePluginInstallPolicy,
pub authentication: MarketplacePluginAuthPolicy,
// TODO: Surface or enforce product gating at the Codex/plugin consumer boundary instead of
// only carrying it through core marketplace metadata.
pub products: Vec<Product>,
}
codex-rs/core/src/skills/mod.rs
-2
View File
@@ -20,6 +20,4 @@ pub use model::SkillError;
pub use model::SkillLoadOutcome;
pub use model::SkillMetadata;
pub use model::SkillPolicy;
pub use model::filter_skill_load_outcome_for_session_source;
pub use model::filter_skills_for_session_source;
pub use render::render_skills_section;
codex-rs/core/src/skills/model.rs
+2 -44
View File
@@ -5,7 +5,6 @@ use std::sync::Arc;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SkillScope;
use serde::Deserialize;
@@ -43,18 +42,13 @@ impl SkillMetadata {
.and_then(|policy| policy.allow_implicit_invocation)
.unwrap_or(true)
}
pub fn matches_product_restriction(&self, session_source: &SessionSource) -> bool {
match &self.policy {
Some(policy) => session_source.matches_product_restriction(&policy.products),
None => true,
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub struct SkillPolicy {
pub allow_implicit_invocation: Option<bool>,
// TODO: Enforce product gating in Codex skill selection/injection instead of only parsing and
// storing this metadata.
pub products: Vec<Product>,
}
@@ -121,39 +115,3 @@ impl SkillLoadOutcome {
.map(|skill| (skill, self.is_skill_enabled(skill)))
}
}
pub fn filter_skill_load_outcome_for_session_source(
mut outcome: SkillLoadOutcome,
session_source: &SessionSource,
) -> SkillLoadOutcome {
outcome
.skills
.retain(|skill| skill.matches_product_restriction(session_source));
outcome.implicit_skills_by_scripts_dir = Arc::new(
outcome
.implicit_skills_by_scripts_dir
.iter()
.filter(|(_, skill)| skill.matches_product_restriction(session_source))
.map(|(path, skill)| (path.clone(), skill.clone()))
.collect(),
);
outcome.implicit_skills_by_doc_path = Arc::new(
outcome
.implicit_skills_by_doc_path
.iter()
.filter(|(_, skill)| skill.matches_product_restriction(session_source))
.map(|(path, skill)| (path.clone(), skill.clone()))
.collect(),
);
outcome
}
pub fn filter_skills_for_session_source(
skills: Vec<SkillMetadata>,
session_source: &SessionSource,
) -> Vec<SkillMetadata> {
skills
.into_iter()
.filter(|skill| skill.matches_product_restriction(session_source))
.collect()
}