feat: enforce managed skill requirements

Co-authored-by: Codex noreply@openai.com

codex-rs/core-skills/src/manager.rs

@@ -4,6 +4,7 @@ use std::sync::Arc;
 use std::sync::RwLock;
 
 use codex_config::ConfigLayerStack;
+use codex_config::SkillSourceRequirement;
 use codex_exec_server::ExecutorFileSystem;
 use codex_protocol::protocol::Product;
 use codex_protocol::protocol::SkillScope;
@@ -51,7 +52,7 @@ impl SkillsLoadInput {
 pub struct SkillsManager {
     codex_home: AbsolutePathBuf,
     restriction_product: Option<Product>,
-    cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, SkillLoadOutcome>>,
+    cache_by_cwd: RwLock<HashMap<CwdSkillsCacheKey, SkillLoadOutcome>>,
     cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
 }
 
@@ -123,6 +124,7 @@ impl SkillsManager {
         if !input.bundled_skills_enabled {
             roots.retain(|root| root.scope != SkillScope::System);
         }
+        retain_allowed_skill_roots(&mut roots, &input.config_layer_stack);
         roots
     }
 
@@ -144,9 +146,10 @@ impl SkillsManager {
         fs: Option<Arc<dyn ExecutorFileSystem>>,
     ) -> SkillLoadOutcome {
         let use_cwd_cache = fs.is_some();
+        let cache_key = cwd_skills_cache_key(&input.cwd, &input.config_layer_stack);
         if use_cwd_cache
             && !force_reload
-            && let Some(outcome) = self.cached_outcome_for_cwd(&input.cwd)
+            && let Some(outcome) = self.cached_outcome_for_cwd(&cache_key)
         {
             return outcome;
         }
@@ -173,6 +176,7 @@ impl SkillsManager {
                     }),
             );
         }
+        retain_allowed_skill_roots(&mut roots, &input.config_layer_stack);
         let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
         let outcome = self.build_skill_outcome(roots, &skill_config_rules).await;
         if use_cwd_cache {
@@ -180,7 +184,7 @@ impl SkillsManager {
                 .cache_by_cwd
                 .write()
                 .unwrap_or_else(std::sync::PoisonError::into_inner);
-            cache.insert(input.cwd.clone(), outcome.clone());
+            cache.insert(cache_key, outcome.clone());
         }
         outcome
     }
@@ -221,10 +225,10 @@ impl SkillsManager {
         info!("skills cache cleared ({cleared} entries)");
     }
 
-    fn cached_outcome_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option<SkillLoadOutcome> {
+    fn cached_outcome_for_cwd(&self, cache_key: &CwdSkillsCacheKey) -> Option<SkillLoadOutcome> {
         match self.cache_by_cwd.read() {
-            Ok(cache) => cache.get(cwd).cloned(),
-            Err(err) => err.into_inner().get(cwd).cloned(),
+            Ok(cache) => cache.get(cache_key).cloned(),
+            Err(err) => err.into_inner().get(cache_key).cloned(),
         }
     }
 
@@ -239,12 +243,43 @@ impl SkillsManager {
     }
 }
 
+fn retain_allowed_skill_roots(roots: &mut Vec<SkillRoot>, config_layer_stack: &ConfigLayerStack) {
+    let Some(requirements) = config_layer_stack.requirements().skills.as_ref() else {
+        return;
+    };
+
+    roots.retain(|root| {
+        requirements
+            .value
+            .allows_source(skill_source_for_root(root))
+    });
+}
+
+fn skill_source_for_root(root: &SkillRoot) -> SkillSourceRequirement {
+    if root.plugin_id.is_some() {
+        return SkillSourceRequirement::Plugin;
+    }
+
+    match root.scope {
+        SkillScope::Repo => SkillSourceRequirement::Repo,
+        SkillScope::User => SkillSourceRequirement::User,
+        SkillScope::System => SkillSourceRequirement::System,
+        SkillScope::Admin => SkillSourceRequirement::Admin,
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 struct ConfigSkillsCacheKey {
     roots: Vec<(AbsolutePathBuf, u8, Option<String>)>,
     skill_config_rules: SkillConfigRules,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+struct CwdSkillsCacheKey {
+    cwd: AbsolutePathBuf,
+    allowed_sources: Option<Vec<SkillSourceRequirement>>,
+}
+
 pub fn bundled_skills_enabled_from_stack(
     config_layer_stack: &codex_config::ConfigLayerStack,
 ) -> bool {
@@ -288,6 +323,20 @@ fn config_skills_cache_key(
     }
 }
 
+fn cwd_skills_cache_key(
+    cwd: &AbsolutePathBuf,
+    config_layer_stack: &ConfigLayerStack,
+) -> CwdSkillsCacheKey {
+    CwdSkillsCacheKey {
+        cwd: cwd.clone(),
+        allowed_sources: config_layer_stack
+            .requirements()
+            .skills
+            .as_ref()
+            .and_then(|requirements| requirements.value.allowed_sources.clone()),
+    }
+}
+
 fn finalize_skill_outcome(
     mut outcome: SkillLoadOutcome,
     disabled_paths: HashSet<AbsolutePathBuf>,

codex-rs/core-skills/src/manager_tests.rs

@@ -6,7 +6,12 @@ use codex_app_server_protocol::ConfigLayerSource;
 use codex_config::CONFIG_TOML_FILE;
 use codex_config::ConfigLayerEntry;
 use codex_config::ConfigLayerStack;
+use codex_config::ConfigRequirements;
 use codex_config::ConfigRequirementsToml;
+use codex_config::RequirementSource;
+use codex_config::SkillSourceRequirement;
+use codex_config::SkillsRequirementsToml;
+use codex_config::Sourced;
 use codex_exec_server::LOCAL_FS;
 use codex_utils_absolute_path::AbsolutePathBuf;
 use codex_utils_absolute_path::test_support::PathBufExt;
@@ -27,6 +32,13 @@ fn write_user_skill(codex_home: &TempDir, dir: &str, name: &str, description: &s
     fs::write(skill_dir.join("SKILL.md"), content).unwrap();
 }
 
+fn write_repo_skill(cwd: &TempDir, dir: &str, name: &str, description: &str) {
+    let skill_dir = cwd.path().join(".agents/skills").join(dir);
+    fs::create_dir_all(&skill_dir).unwrap();
+    let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
+    fs::write(skill_dir.join("SKILL.md"), content).unwrap();
+}
+
 fn write_plugin_skill(
     codex_home: &TempDir,
     marketplace: &str,
@@ -94,9 +106,17 @@ fn user_config_layer(codex_home: &TempDir, config_toml: &str) -> ConfigLayerEntr
 }
 
 fn config_stack(codex_home: &TempDir, user_config_toml: &str) -> ConfigLayerStack {
+    config_stack_with_requirements(codex_home, user_config_toml, ConfigRequirements::default())
+}
+
+fn config_stack_with_requirements(
+    codex_home: &TempDir,
+    user_config_toml: &str,
+    requirements: ConfigRequirements,
+) -> ConfigLayerStack {
     ConfigLayerStack::new(
         vec![user_config_layer(codex_home, user_config_toml)],
-        Default::default(),
+        requirements,
         ConfigRequirementsToml::default(),
     )
     .expect("valid config layer stack")
@@ -262,6 +282,65 @@ async fn skills_for_config_disables_plugin_skills_by_name() {
     );
 }
 
+#[tokio::test]
+async fn skills_for_config_filters_disallowed_managed_sources() {
+    let codex_home = tempfile::tempdir().expect("tempdir");
+    let cwd = tempfile::tempdir().expect("tempdir");
+    write_user_skill(&codex_home, "user", "user-skill", "from user");
+    write_repo_skill(&cwd, "repo", "repo-skill", "from repo");
+    let plugin_skill_path = write_plugin_skill(
+        &codex_home,
+        "test",
+        "sample",
+        "plugin",
+        "plugin-skill",
+        "from plugin",
+    );
+    let config_layer_stack = config_stack_with_requirements(
+        &codex_home,
+        "",
+        ConfigRequirements {
+            skills: Some(Sourced::new(
+                SkillsRequirementsToml {
+                    allowed_sources: Some(vec![
+                        SkillSourceRequirement::System,
+                        SkillSourceRequirement::Admin,
+                        SkillSourceRequirement::Plugin,
+                    ]),
+                },
+                RequirementSource::Unknown,
+            )),
+            ..Default::default()
+        },
+    );
+    let skills_manager = SkillsManager::new(
+        codex_home.path().abs(),
+        /*bundled_skills_enabled*/ true,
+    );
+
+    let outcome = skills_for_config_with_stack(
+        &skills_manager,
+        &cwd,
+        &config_layer_stack,
+        &[plugin_skill_path
+            .parent()
+            .and_then(std::path::Path::parent)
+            .expect("plugin skills root")
+            .to_path_buf()
+            .abs()],
+    )
+    .await;
+    let skill_names = outcome
+        .skills
+        .iter()
+        .map(|skill| skill.name.as_str())
+        .collect::<HashSet<_>>();
+
+    assert!(skill_names.contains("sample:plugin-skill"));
+    assert!(!skill_names.contains("user-skill"));
+    assert!(!skill_names.contains("repo-skill"));
+}
+
 #[tokio::test]
 async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
     let codex_home = tempfile::tempdir().expect("tempdir");
@@ -322,6 +401,71 @@ async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
     assert_eq!(outcome_without_extra.errors, outcome_with_extra.errors);
 }
 
+#[tokio::test]
+async fn skills_for_cwd_separates_cache_entries_by_managed_allowed_sources() {
+    let codex_home = tempfile::tempdir().expect("tempdir");
+    let cwd = tempfile::tempdir().expect("tempdir");
+    write_user_skill(&codex_home, "user", "user-skill", "from user");
+    let unrestricted_stack = config_stack(&codex_home, "");
+    let restricted_stack = config_stack_with_requirements(
+        &codex_home,
+        "",
+        ConfigRequirements {
+            skills: Some(Sourced::new(
+                SkillsRequirementsToml {
+                    allowed_sources: Some(vec![SkillSourceRequirement::System]),
+                },
+                RequirementSource::Unknown,
+            )),
+            ..Default::default()
+        },
+    );
+    let skills_manager = SkillsManager::new(
+        codex_home.path().abs(),
+        /*bundled_skills_enabled*/ true,
+    );
+
+    let unrestricted_input = SkillsLoadInput::new(
+        cwd.path().abs(),
+        Vec::new(),
+        unrestricted_stack.clone(),
+        bundled_skills_enabled_from_stack(&unrestricted_stack),
+    );
+    let unrestricted = skills_manager
+        .skills_for_cwd(
+            &unrestricted_input,
+            /*force_reload*/ false,
+            Some(Arc::clone(&LOCAL_FS)),
+        )
+        .await;
+    assert!(
+        unrestricted
+            .skills
+            .iter()
+            .any(|skill| skill.name == "user-skill")
+    );
+
+    let restricted_input = SkillsLoadInput::new(
+        cwd.path().abs(),
+        Vec::new(),
+        restricted_stack.clone(),
+        bundled_skills_enabled_from_stack(&restricted_stack),
+    );
+    let restricted = skills_manager
+        .skills_for_cwd(
+            &restricted_input,
+            /*force_reload*/ false,
+            Some(Arc::clone(&LOCAL_FS)),
+        )
+        .await;
+    assert!(
+        !restricted
+            .skills
+            .iter()
+            .any(|skill| skill.name == "user-skill")
+    );
+}
+
 #[tokio::test]
 async fn skills_for_cwd_loads_repo_user_and_extra_roots_with_local_fs() {
     let codex_home = tempfile::tempdir().expect("tempdir");