[codex] Validate plugin skill base names (#25782)

## Summary

- Validate skill base name length before plugin namespacing.
- Bound the composed `plugin:skill` qualified name to 128 characters.
- Keep plugin skill runtime names in the existing `plugin:skill` form.
- Add regression tests for the max qualified-name boundary and rejection
path.

## Root Cause

Plugin skills are represented as `plugin_name:skill_name`, but the
loader previously applied the 64-character skill name limit after adding
the plugin namespace. Moving that check to the base name fixes valid
plugin skills with longer namespaces, and the separate 128-character
qualified-name limit keeps model-visible skill names bounded.

## Validation

- `just fmt`
- `just test -p codex-core-skills plugin_skill_name_length_limit`
- `git diff --check`
This commit is contained in:
xl-openai
2026-06-01 23:33:02 -07:00
committed by GitHub
Unverified
parent 07f04cc3c7
commit 67b805fc11
2 changed files with 81 additions and 1 deletions
+3 -1
View File
@@ -110,6 +110,7 @@ const SKILLS_METADATA_DIR: &str = "agents";
const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
const SKILLS_DIR_NAME: &str = "skills";
const MAX_NAME_LEN: usize = 64;
const MAX_QUALIFIED_NAME_LEN: usize = 128;
const MAX_DESCRIPTION_LEN: usize = 1024;
const MAX_SHORT_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN;
const MAX_DEFAULT_PROMPT_LEN: usize = MAX_DESCRIPTION_LEN;
@@ -660,7 +661,8 @@ async fn parse_skill_file(
policy,
} = load_skill_metadata(fs, path, plugin_root).await;
validate_len(&name, MAX_NAME_LEN, "name")?;
validate_len(&base_name, MAX_NAME_LEN, "name")?;
validate_len(&name, MAX_QUALIFIED_NAME_LEN, "qualified name")?;
validate_len(&description, MAX_DESCRIPTION_LEN, "description")?;
if let Some(short_description) = short_description.as_deref() {
validate_len(
+78
View File
@@ -1301,6 +1301,84 @@ async fn namespaces_plugin_skills_using_plugin_name() {
);
}
#[tokio::test]
async fn plugin_skill_name_length_limit_allows_max_qualified_name() {
let root = tempfile::tempdir().expect("tempdir");
let plugin_name = "p".repeat(MAX_NAME_LEN - 1);
let skill_name = "s".repeat(MAX_NAME_LEN);
let plugin_root = root.path().join("plugins").join(&plugin_name);
let frontmatter = format!("name: {skill_name}\ndescription: search sample data");
let skill_path = write_raw_skill_at(&plugin_root.join("skills"), "sample-search", &frontmatter);
fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap();
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
format!(r#"{{"name":"{plugin_name}"}}"#),
)
.unwrap();
let outcome = load_skills_from_roots([SkillRoot {
path: plugin_root.join("skills").abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("sample@test".to_string()),
plugin_root: Some(plugin_root.abs()),
}])
.await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![SkillMetadata {
name: format!("{plugin_name}:{skill_name}"),
description: "search sample data".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: Some("sample@test".to_string()),
}]
);
}
#[tokio::test]
async fn plugin_skill_name_length_limit_rejects_overlong_qualified_name() {
let root = tempfile::tempdir().expect("tempdir");
let plugin_name = "p".repeat(MAX_NAME_LEN);
let skill_name = "s".repeat(MAX_NAME_LEN);
let plugin_root = root.path().join("plugins").join(&plugin_name);
let frontmatter = format!("name: {skill_name}\ndescription: search sample data");
write_raw_skill_at(&plugin_root.join("skills"), "sample-search", &frontmatter);
fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap();
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
format!(r#"{{"name":"{plugin_name}"}}"#),
)
.unwrap();
let outcome = load_skills_from_roots([SkillRoot {
path: plugin_root.join("skills").abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("sample@test".to_string()),
plugin_root: Some(plugin_root.abs()),
}])
.await;
assert_eq!(outcome.skills, Vec::new());
assert_eq!(outcome.errors.len(), 1);
assert!(
outcome.errors[0].message.contains("invalid qualified name"),
"expected qualified name length error, got: {:?}",
outcome.errors
);
}
#[tokio::test]
async fn loads_short_description_from_metadata() {
let codex_home = tempfile::tempdir().expect("tempdir");