mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Support portable Agent Plugins throughout installation (#36544)
## Why Agent Plugins use a schema-declared root `plugin.json` and can have dotted names or versions that do not fit Codex's directory-safe version format. The packaging and installation paths still assumed the legacy manifest layout and identifier rules. ## What changed - Recognize valid root Agent Plugin manifests when discovering, packing, and installing plugins, while leaving unrelated root manifests on the legacy path. - Accept safe dotted plugin names, default missing Agent Plugin versions to `1.0.0`, and derive stable directory-safe versions when necessary without rewriting the portable manifest. - Skip legacy command migration for Agent Plugins and reject symlinks or other unsupported file types while copying plugin sources. ## Testing Add coverage for portable bundle round trips, manifest discovery, dotted names, version handling, command preservation, and symlink rejection. GitOrigin-RevId: 61476c4c4100495842253d8b429c0b896490962d
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -2886,6 +2886,7 @@ dependencies = [
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serde_yaml",
|
||||
"sha2 0.10.9",
|
||||
"tar",
|
||||
"tempfile",
|
||||
"thiserror 2.0.18",
|
||||
|
||||
@@ -45,6 +45,7 @@ semver = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_yaml = { workspace = true }
|
||||
sha2 = { workspace = true }
|
||||
tar = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use super::PluginManifest;
|
||||
use super::PluginManifestMcpServers;
|
||||
use super::parse_resolved_plugin_manifest;
|
||||
use super::load_plugin_manifest;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::AGENT_PLUGIN_SCHEMA_URI;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -23,23 +23,7 @@ fn write_agent_plugin_manifest(plugin_root: &Path, extra_fields: &str) {
|
||||
}
|
||||
|
||||
fn load_manifest(plugin_root: &Path) -> PluginManifest {
|
||||
try_load_manifest(plugin_root).expect("load plugin manifest")
|
||||
}
|
||||
|
||||
fn try_load_manifest(plugin_root: &Path) -> Option<PluginManifest> {
|
||||
let manifest_path = plugin_root.join("plugin.json");
|
||||
let contents = fs::read_to_string(&manifest_path).ok()?;
|
||||
let overlay_path = plugin_root.join(".codex-plugin/plugin.json");
|
||||
let overlay_contents = fs::read_to_string(&overlay_path).ok();
|
||||
parse_resolved_plugin_manifest(
|
||||
plugin_root,
|
||||
&manifest_path,
|
||||
&contents,
|
||||
overlay_contents
|
||||
.as_ref()
|
||||
.map(|contents| (overlay_path.as_path(), contents.as_str())),
|
||||
)
|
||||
.ok()
|
||||
load_plugin_manifest(plugin_root).expect("load plugin manifest")
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -180,21 +164,21 @@ fn rejects_overlong_name_and_wrong_metadata_types() {
|
||||
),
|
||||
)
|
||||
.expect("write manifest");
|
||||
assert_eq!(try_load_manifest(&plugin_root), None);
|
||||
assert_eq!(load_plugin_manifest(&plugin_root), None);
|
||||
|
||||
fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"demo-plugin","homepage":42}}"#),
|
||||
)
|
||||
.expect("write manifest");
|
||||
assert_eq!(try_load_manifest(&plugin_root), None);
|
||||
assert_eq!(load_plugin_manifest(&plugin_root), None);
|
||||
|
||||
fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"demo-plugin","version":null}}"#),
|
||||
)
|
||||
.expect("write manifest");
|
||||
assert_eq!(try_load_manifest(&plugin_root), None);
|
||||
assert_eq!(load_plugin_manifest(&plugin_root), None);
|
||||
|
||||
fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
@@ -203,7 +187,7 @@ fn rejects_overlong_name_and_wrong_metadata_types() {
|
||||
),
|
||||
)
|
||||
.expect("write manifest");
|
||||
assert_eq!(try_load_manifest(&plugin_root), None);
|
||||
assert_eq!(load_plugin_manifest(&plugin_root), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1668,7 +1668,7 @@ fn list_marketplaces_skips_plugins_with_invalid_names_but_keeps_marketplace() {
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "invalid.plugin",
|
||||
"name": "invalid/plugin",
|
||||
"source": {
|
||||
"source": "local",
|
||||
"path": "./invalid-plugin"
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::manifest::load_plugin_manifest;
|
||||
use flate2::Compression;
|
||||
use flate2::read::GzDecoder;
|
||||
use flate2::write::GzEncoder;
|
||||
@@ -59,10 +60,12 @@ pub(crate) fn pack_plugin_bundle_tar_gz(
|
||||
reason: "expected a plugin directory".to_string(),
|
||||
});
|
||||
}
|
||||
if !plugin_path.join(".codex-plugin/plugin.json").is_file() {
|
||||
if !plugin_path.join(".codex-plugin/plugin.json").is_file()
|
||||
&& load_plugin_manifest(plugin_path).is_none()
|
||||
{
|
||||
return Err(PluginBundlePackError::InvalidPluginPath {
|
||||
path: plugin_path.to_path_buf(),
|
||||
reason: "missing .codex-plugin/plugin.json".to_string(),
|
||||
reason: "missing .codex-plugin/plugin.json or valid Agent Plugin manifest".to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -313,3 +316,7 @@ impl fmt::Display for ArchiveSizeLimitExceeded {
|
||||
}
|
||||
|
||||
impl std::error::Error for ArchiveSizeLimitExceeded {}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "plugin_bundle_archive_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
41
codex-rs/core-plugins/src/plugin_bundle_archive_tests.rs
Normal file
41
codex-rs/core-plugins/src/plugin_bundle_archive_tests.rs
Normal file
@@ -0,0 +1,41 @@
|
||||
use super::*;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn portable_root_manifest_can_be_packed_and_unpacked() {
|
||||
let source = tempdir().expect("source tempdir");
|
||||
fs::write(
|
||||
source.path().join("plugin.json"),
|
||||
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"portable"}"#,
|
||||
)
|
||||
.expect("write portable manifest");
|
||||
fs::create_dir_all(source.path().join("skills/demo")).expect("create skill directory");
|
||||
fs::write(source.path().join("skills/demo/SKILL.md"), "# Demo\n").expect("write skill");
|
||||
|
||||
let archive =
|
||||
pack_plugin_bundle_tar_gz(source.path(), 1024 * 1024).expect("pack portable plugin");
|
||||
let destination = tempdir().expect("destination tempdir");
|
||||
unpack_plugin_bundle_tar_gz(&archive, destination.path(), 1024 * 1024)
|
||||
.expect("unpack portable plugin");
|
||||
|
||||
assert!(destination.path().join("plugin.json").is_file());
|
||||
assert!(destination.path().join("skills/demo/SKILL.md").is_file());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_portable_manifest_is_rejected_before_packing() {
|
||||
let source = tempdir().expect("source tempdir");
|
||||
fs::write(
|
||||
source.path().join("plugin.json"),
|
||||
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"UPPER"}"#,
|
||||
)
|
||||
.expect("write invalid portable manifest");
|
||||
|
||||
let error = pack_plugin_bundle_tar_gz(source.path(), 1024 * 1024)
|
||||
.expect_err("invalid Agent Plugin manifest should fail");
|
||||
|
||||
assert!(matches!(
|
||||
error,
|
||||
PluginBundlePackError::InvalidPluginPath { .. }
|
||||
));
|
||||
}
|
||||
@@ -313,6 +313,33 @@ async fn standalone_capability_root_is_not_a_plugin() {
|
||||
assert_eq!(resolved, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn root_agent_plugin_manifest_is_not_an_executor_plugin() {
|
||||
let temp_dir = tempdir().expect("tempdir");
|
||||
let plugin_root = temp_dir.path().join("agent-plugin");
|
||||
write_manifest(
|
||||
&plugin_root,
|
||||
"plugin.json",
|
||||
r#"{
|
||||
"$schema": "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json",
|
||||
"name": "agent-plugin",
|
||||
"version": "1.0.0"
|
||||
}"#,
|
||||
);
|
||||
let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests()));
|
||||
|
||||
let resolved = provider
|
||||
.resolve(&selected_root(
|
||||
"agent-plugin",
|
||||
LOCAL_ENVIRONMENT_ID,
|
||||
&plugin_root,
|
||||
))
|
||||
.await
|
||||
.expect("resolve selected root");
|
||||
|
||||
assert_eq!(resolved, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unavailable_environment_does_not_fall_back_to_host_filesystem() {
|
||||
let temp_dir = tempdir().expect("tempdir");
|
||||
|
||||
@@ -12,6 +12,9 @@ use codex_http_client::RouteAwareRequestError;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_plugin::PluginIdError;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::AGENT_PLUGIN_MANIFEST_RELATIVE_PATH;
|
||||
use codex_utils_plugins::AgentPluginSchemaStatus;
|
||||
use codex_utils_plugins::agent_plugin_schema_status;
|
||||
use codex_utils_plugins::find_plugin_manifest_path;
|
||||
use http::Method;
|
||||
use http::StatusCode;
|
||||
@@ -524,6 +527,11 @@ fn overwrite_plugin_manifest_version(
|
||||
let contents = fs::read_to_string(&manifest_path).map_err(|source| {
|
||||
RemotePluginBundleInstallError::io("failed to read remote plugin manifest", source)
|
||||
})?;
|
||||
if manifest_path == plugin_root.join(AGENT_PLUGIN_MANIFEST_RELATIVE_PATH)
|
||||
&& agent_plugin_schema_status(&contents) == AgentPluginSchemaStatus::Supported
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
let mut manifest: JsonValue = serde_json::from_str(&contents).map_err(|err| {
|
||||
RemotePluginBundleInstallError::InvalidBundle(format!(
|
||||
"failed to parse remote plugin manifest: {err}"
|
||||
@@ -644,6 +652,22 @@ mod tests {
|
||||
|
||||
const REMOTE_PLUGIN_ID: &str = "plugins~Plugin_00000000000000000000000000000000";
|
||||
|
||||
#[test]
|
||||
fn remote_version_normalization_preserves_portable_root_manifest() {
|
||||
let temp_dir = tempdir().expect("tempdir");
|
||||
let manifest_path = temp_dir.path().join("plugin.json");
|
||||
let original = r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"portable","version":"release-2026-07"}"#;
|
||||
fs::write(&manifest_path, original).expect("write portable manifest");
|
||||
|
||||
overwrite_plugin_manifest_version(temp_dir.path(), "1.2.3")
|
||||
.expect("prepare portable remote plugin");
|
||||
|
||||
assert_eq!(
|
||||
fs::read_to_string(manifest_path).expect("read portable manifest"),
|
||||
original
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_remote_plugin_bundle_uses_detail_name_for_local_plugin_id() {
|
||||
let bundle = validate_remote_plugin_bundle(
|
||||
|
||||
@@ -5,11 +5,15 @@ use crate::manifest::parse_plugin_manifest;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_plugin::validate_plugin_segment;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::AgentPluginSchemaStatus;
|
||||
use codex_utils_plugins::agent_plugin_schema_status;
|
||||
use codex_utils_plugins::find_plugin_manifest_path;
|
||||
use semver::Version;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value as JsonValue;
|
||||
use sha2::Digest;
|
||||
use sha2::Sha256;
|
||||
use std::cmp::Ordering;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
@@ -22,6 +26,7 @@ pub const PLUGINS_CACHE_DIR: &str = "plugins/cache";
|
||||
pub const PLUGINS_DATA_DIR: &str = "plugins/data";
|
||||
const REMOTE_PLUGIN_INSTALL_METADATA_FILE: &str = ".codex-remote-plugin-install.json";
|
||||
const REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION: u8 = 1;
|
||||
const DEFAULT_AGENT_PLUGIN_VERSION: &str = "1.0.0";
|
||||
|
||||
#[derive(Debug, Deserialize, Serialize)]
|
||||
struct RemotePluginInstallMetadata {
|
||||
@@ -434,10 +439,36 @@ fn plugin_version_for_install_manifest(
|
||||
source_path: &Path,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<String, PluginStoreError> {
|
||||
let plugin_version = plugin_manifest_version_for_source(source_path, manifest)?
|
||||
.unwrap_or_else(|| DEFAULT_PLUGIN_VERSION.to_string());
|
||||
validate_plugin_version_segment(&plugin_version).map_err(PluginStoreError::Invalid)?;
|
||||
Ok(plugin_version)
|
||||
let (plugin_version, is_agent_plugin) =
|
||||
plugin_manifest_version_for_source(source_path, manifest)?;
|
||||
let plugin_version = plugin_version.unwrap_or_else(|| {
|
||||
if is_agent_plugin {
|
||||
DEFAULT_AGENT_PLUGIN_VERSION.to_string()
|
||||
} else {
|
||||
DEFAULT_PLUGIN_VERSION.to_string()
|
||||
}
|
||||
});
|
||||
match validate_plugin_version_segment(&plugin_version) {
|
||||
Ok(()) => Ok(plugin_version),
|
||||
Err(_) if is_agent_plugin => {
|
||||
let digest = Sha256::digest(plugin_version.as_bytes());
|
||||
Ok(format!(
|
||||
"agent-plugins-{}",
|
||||
hex_prefix(&digest, /*count*/ 12)
|
||||
))
|
||||
}
|
||||
Err(message) => Err(PluginStoreError::Invalid(message)),
|
||||
}
|
||||
}
|
||||
|
||||
fn hex_prefix(bytes: &[u8], count: usize) -> String {
|
||||
const HEX: &[u8; 16] = b"0123456789abcdef";
|
||||
let mut output = String::with_capacity(count.saturating_mul(2));
|
||||
for byte in bytes.iter().take(count) {
|
||||
output.push(HEX[usize::from(byte >> 4)] as char);
|
||||
output.push(HEX[usize::from(byte & 0x0f)] as char);
|
||||
}
|
||||
output
|
||||
}
|
||||
|
||||
pub fn validate_plugin_version_segment(plugin_version: &str) -> Result<(), String> {
|
||||
@@ -485,7 +516,7 @@ struct RawPluginManifestVersion {
|
||||
fn plugin_manifest_version_for_source(
|
||||
source_path: &Path,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<Option<String>, PluginStoreError> {
|
||||
) -> Result<(Option<String>, bool), PluginStoreError> {
|
||||
let contents = match manifest {
|
||||
InstallManifest::OnDisk => {
|
||||
let manifest_path = find_plugin_manifest_path(source_path)
|
||||
@@ -495,23 +526,29 @@ fn plugin_manifest_version_for_source(
|
||||
}
|
||||
InstallManifest::Fallback(contents) => contents.to_string(),
|
||||
};
|
||||
let is_agent_plugin =
|
||||
agent_plugin_schema_status(&contents) == AgentPluginSchemaStatus::Supported;
|
||||
let manifest: RawPluginManifestVersion = serde_json::from_str(&contents)
|
||||
.map_err(|err| PluginStoreError::Invalid(format!("failed to parse plugin.json: {err}")))?;
|
||||
let Some(version) = manifest.version else {
|
||||
return Ok(None);
|
||||
return Ok((None, is_agent_plugin));
|
||||
};
|
||||
let Some(version) = version.as_str() else {
|
||||
return Err(PluginStoreError::Invalid(
|
||||
"invalid plugin version in plugin.json: expected string".to_string(),
|
||||
));
|
||||
};
|
||||
if is_agent_plugin {
|
||||
let version = version.trim();
|
||||
return Ok(((!version.is_empty()).then(|| version.to_string()), true));
|
||||
}
|
||||
let version = version.trim();
|
||||
if version.is_empty() {
|
||||
return Err(PluginStoreError::Invalid(
|
||||
"invalid plugin version in plugin.json: must not be blank".to_string(),
|
||||
));
|
||||
}
|
||||
Ok(Some(version.to_string()))
|
||||
Ok((Some(version.to_string()), false))
|
||||
}
|
||||
|
||||
fn plugin_name_for_source(
|
||||
@@ -588,7 +625,12 @@ fn replace_plugin_root_atomically(
|
||||
fs::write(&manifest_path, contents)
|
||||
.map_err(|err| PluginStoreError::io("failed to write fallback plugin manifest", err))?;
|
||||
}
|
||||
if let Err(err) = migrate_plugin_commands(&staged_version_root) {
|
||||
let is_agent_plugin = fs::read_to_string(staged_version_root.join("plugin.json"))
|
||||
.ok()
|
||||
.is_some_and(|contents| {
|
||||
agent_plugin_schema_status(&contents) == AgentPluginSchemaStatus::Supported
|
||||
});
|
||||
if !is_agent_plugin && let Err(err) = migrate_plugin_commands(&staged_version_root) {
|
||||
tracing::warn!(%err, "failed to migrate plugin commands into skills");
|
||||
}
|
||||
|
||||
@@ -703,6 +745,16 @@ fn copy_dir_recursive(source: &Path, target: &Path) -> Result<(), PluginStoreErr
|
||||
} else if file_type.is_file() {
|
||||
fs::copy(&source_path, &target_path)
|
||||
.map_err(|err| PluginStoreError::io("failed to copy plugin file", err))?;
|
||||
} else if file_type.is_symlink() {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin source contains unsupported symbolic link: {}",
|
||||
source_path.display()
|
||||
)));
|
||||
} else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin source contains unsupported file type: {}",
|
||||
source_path.display()
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -350,6 +350,104 @@ fn install_rejects_blank_manifest_version() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_plugin_blank_version_uses_default_version() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let plugin_root = tmp.path().join("agent-plugin");
|
||||
fs::create_dir_all(&plugin_root).unwrap();
|
||||
fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"agent-plugin","version":" "}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let plugin_id = PluginId::new("agent-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
|
||||
let result = PluginStore::new(tmp.path().to_path_buf())
|
||||
.install(AbsolutePathBuf::try_from(plugin_root).unwrap(), plugin_id)
|
||||
.expect("install Agent Plugin");
|
||||
|
||||
assert_eq!(result.plugin_version, DEFAULT_AGENT_PLUGIN_VERSION);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_plugin_install_does_not_migrate_commands() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let plugin_root = tmp.path().join("agent-plugin");
|
||||
fs::create_dir_all(plugin_root.join("commands")).unwrap();
|
||||
fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"agent-plugin","commands":"./commands"}"#,
|
||||
)
|
||||
.unwrap();
|
||||
fs::write(plugin_root.join("commands/demo.md"), "# Demo").unwrap();
|
||||
let plugin_id = PluginId::new("agent-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
|
||||
let result = PluginStore::new(tmp.path().to_path_buf())
|
||||
.install(AbsolutePathBuf::try_from(plugin_root).unwrap(), plugin_id)
|
||||
.expect("install Agent Plugin");
|
||||
|
||||
assert!(
|
||||
!result
|
||||
.installed_path
|
||||
.join(".codex-plugin/migrated-command-skills")
|
||||
.exists()
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn agent_plugin_install_rejects_symlinked_skill_file() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let plugin_root = tmp.path().join("agent-plugin");
|
||||
let skill_root = plugin_root.join("skills/greet");
|
||||
fs::create_dir_all(&skill_root).unwrap();
|
||||
fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"agent-plugin"}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let outside_skill = tmp.path().join("outside-SKILL.md");
|
||||
fs::write(&outside_skill, "---\nname: greet\n---\n").unwrap();
|
||||
std::os::unix::fs::symlink(&outside_skill, skill_root.join("SKILL.md")).unwrap();
|
||||
let plugin_id = PluginId::new("agent-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
|
||||
let err = PluginStore::new(tmp.path().to_path_buf())
|
||||
.install(AbsolutePathBuf::try_from(plugin_root).unwrap(), plugin_id)
|
||||
.expect_err("symlinked Agent Plugin skill should be rejected");
|
||||
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("plugin source contains unsupported symbolic link")
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn agent_plugin_install_rejects_symlinked_executable() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let plugin_root = tmp.path().join("agent-plugin");
|
||||
let bin_root = plugin_root.join("bin");
|
||||
fs::create_dir_all(&bin_root).unwrap();
|
||||
fs::write(
|
||||
plugin_root.join("plugin.json"),
|
||||
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"agent-plugin"}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let outside_executable = tmp.path().join("outside-tool");
|
||||
fs::write(&outside_executable, "#!/bin/sh\n").unwrap();
|
||||
std::os::unix::fs::symlink(&outside_executable, bin_root.join("tool")).unwrap();
|
||||
let plugin_id = PluginId::new("agent-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
|
||||
let err = PluginStore::new(tmp.path().to_path_buf())
|
||||
.install(AbsolutePathBuf::try_from(plugin_root).unwrap(), plugin_id)
|
||||
.expect_err("symlinked Agent Plugin executable should be rejected");
|
||||
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("plugin source contains unsupported symbolic link")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_plugin_version_reads_version_directory_name() {
|
||||
let tmp = tempdir().unwrap();
|
||||
@@ -490,7 +588,7 @@ fn plugin_root_rejects_path_separators_in_key_segments() {
|
||||
let err = PluginId::parse("../../etc@debug").unwrap_err();
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"invalid plugin name: only ASCII letters, digits, `_`, and `-` are allowed in `../../etc@debug`"
|
||||
"invalid plugin name: dots must separate non-empty name segments in `../../etc@debug`"
|
||||
);
|
||||
|
||||
let err = PluginId::parse("sample@../../etc").unwrap_err();
|
||||
@@ -514,7 +612,7 @@ fn install_rejects_manifest_names_with_path_separators() {
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"invalid plugin name: only ASCII letters, digits, `_`, and `-` are allowed"
|
||||
"invalid plugin name: dots must separate non-empty name segments"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -52,13 +52,32 @@ pub fn validate_plugin_segment(segment: &str, kind: &str) -> Result<(), String>
|
||||
if segment.is_empty() {
|
||||
return Err(format!("invalid {kind}: must not be empty"));
|
||||
}
|
||||
if !segment
|
||||
.chars()
|
||||
.all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_')
|
||||
let allow_dots = kind == "plugin name";
|
||||
if allow_dots && matches!(segment, "." | "..") {
|
||||
return Err(format!("invalid {kind}: path traversal is not allowed"));
|
||||
}
|
||||
if allow_dots && (segment.starts_with('.') || segment.ends_with('.') || segment.contains(".."))
|
||||
{
|
||||
return Err(format!(
|
||||
"invalid {kind}: only ASCII letters, digits, `_`, and `-` are allowed"
|
||||
"invalid {kind}: dots must separate non-empty name segments"
|
||||
));
|
||||
}
|
||||
if !segment
|
||||
.chars()
|
||||
.all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_') || allow_dots && ch == '.')
|
||||
{
|
||||
let allowed_characters = if allow_dots {
|
||||
"ASCII letters, digits, `.`, `_`, and `-`"
|
||||
} else {
|
||||
"ASCII letters, digits, `_`, and `-`"
|
||||
};
|
||||
return Err(format!(
|
||||
"invalid {kind}: only {allowed_characters} are allowed"
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "plugin_id_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
34
codex-rs/plugin/src/plugin_id_tests.rs
Normal file
34
codex-rs/plugin/src/plugin_id_tests.rs
Normal file
@@ -0,0 +1,34 @@
|
||||
use super::PluginId;
|
||||
|
||||
#[test]
|
||||
fn accepts_dotted_plugin_names() {
|
||||
let plugin_id =
|
||||
PluginId::new("acme.tools".to_string(), "marketplace".to_string()).expect("plugin id");
|
||||
|
||||
assert_eq!(plugin_id.as_key(), "acme.tools@marketplace");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn marketplace_names_preserve_legacy_character_set() {
|
||||
PluginId::new("acme".to_string(), "marketplace_name-1".to_string()).expect("marketplace name");
|
||||
|
||||
let err = PluginId::new("acme".to_string(), "market.place".to_string())
|
||||
.expect_err("dotted marketplace name");
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"invalid marketplace name: only ASCII letters, digits, `_`, and `-` are allowed"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_dot_path_segments() {
|
||||
assert!(PluginId::new(".".to_string(), "marketplace".to_string()).is_err());
|
||||
assert!(PluginId::new("plugin".to_string(), "..".to_string()).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_dots_that_can_alias_path_segments() {
|
||||
for plugin_name in [".plugin", "plugin.", "plugin..name"] {
|
||||
assert!(PluginId::new(plugin_name.to_string(), "marketplace".to_string()).is_err());
|
||||
}
|
||||
}
|
||||
@@ -8,6 +8,8 @@ use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub const AGENT_PLUGIN_MANIFEST_RELATIVE_PATH: &str = "plugin.json";
|
||||
/// Published Agent Plugins v1 manifest schema:
|
||||
/// https://github.com/agentplugins/agent-plugins-spec/blob/main/schemas/1.0.0/plugin.schema.json
|
||||
pub const AGENT_PLUGIN_SCHEMA_URI: &str =
|
||||
"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json";
|
||||
pub const SUPPORTED_AGENT_PLUGIN_SCHEMA_URIS: &[&str] = &[AGENT_PLUGIN_SCHEMA_URI];
|
||||
@@ -37,6 +39,25 @@ pub fn agent_plugin_schema_status(contents: &str) -> AgentPluginSchemaStatus {
|
||||
}
|
||||
|
||||
pub fn find_plugin_manifest_path(plugin_root: &Path) -> Option<PathBuf> {
|
||||
let agent_manifest_path = plugin_root.join(AGENT_PLUGIN_MANIFEST_RELATIVE_PATH);
|
||||
match std::fs::symlink_metadata(&agent_manifest_path) {
|
||||
Ok(metadata) if metadata.file_type().is_symlink() || !metadata.file_type().is_file() => {
|
||||
return None;
|
||||
}
|
||||
Ok(_) => {
|
||||
if std::fs::read_to_string(&agent_manifest_path)
|
||||
.ok()
|
||||
.is_some_and(|contents| {
|
||||
agent_plugin_schema_status(&contents) != AgentPluginSchemaStatus::Unrelated
|
||||
})
|
||||
{
|
||||
return Some(agent_manifest_path);
|
||||
}
|
||||
}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(_) => return None,
|
||||
}
|
||||
|
||||
DISCOVERABLE_PLUGIN_MANIFEST_PATHS
|
||||
.iter()
|
||||
.map(|relative_path| plugin_root.join(relative_path))
|
||||
@@ -105,6 +126,8 @@ pub async fn plugin_namespace_for_skill_uri(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::AGENT_PLUGIN_MANIFEST_RELATIVE_PATH;
|
||||
use super::AGENT_PLUGIN_SCHEMA_URI;
|
||||
use super::find_plugin_manifest_path;
|
||||
use super::plugin_namespace_for_skill_path;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
@@ -176,6 +199,22 @@ mod tests {
|
||||
assert_eq!(find_plugin_manifest_path(&plugin_root), Some(manifest_path));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recognizes_schema_declared_root_plugin_manifest() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let plugin_root = tmp.path().join("plugins/portable");
|
||||
let skill_path = plugin_root.join("skills/search/SKILL.md");
|
||||
fs::create_dir_all(skill_path.parent().expect("parent")).expect("mkdir");
|
||||
let manifest_path = plugin_root.join(AGENT_PLUGIN_MANIFEST_RELATIVE_PATH);
|
||||
fs::write(
|
||||
&manifest_path,
|
||||
format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"portable"}}"#),
|
||||
)
|
||||
.expect("write manifest");
|
||||
|
||||
assert_eq!(find_plugin_manifest_path(&plugin_root), Some(manifest_path));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_unrelated_root_plugin_manifest_before_legacy_fallback() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
@@ -189,6 +228,39 @@ mod tests {
|
||||
assert_eq!(find_plugin_manifest_path(&plugin_root), Some(legacy_path));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_nonregular_root_plugin_manifest() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let plugin_root = tmp.path().join("plugins/sample");
|
||||
let legacy_path = plugin_root.join(".codex-plugin/plugin.json");
|
||||
fs::create_dir_all(plugin_root.join("plugin.json")).expect("root manifest directory");
|
||||
fs::create_dir_all(legacy_path.parent().expect("parent")).expect("legacy parent");
|
||||
fs::write(&legacy_path, r#"{"name":"sample"}"#).expect("legacy manifest");
|
||||
|
||||
assert_eq!(find_plugin_manifest_path(&plugin_root), None);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn rejects_symlinked_root_plugin_manifest() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let plugin_root = tmp.path().join("plugins/sample");
|
||||
let manifest_target = tmp.path().join("manifest.json");
|
||||
let legacy_path = plugin_root.join(".codex-plugin/plugin.json");
|
||||
fs::create_dir_all(&plugin_root).expect("plugin root");
|
||||
fs::write(
|
||||
&manifest_target,
|
||||
format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"sample"}}"#),
|
||||
)
|
||||
.expect("manifest target");
|
||||
std::os::unix::fs::symlink(&manifest_target, plugin_root.join("plugin.json"))
|
||||
.expect("root manifest symlink");
|
||||
fs::create_dir_all(legacy_path.parent().expect("parent")).expect("legacy parent");
|
||||
fs::write(&legacy_path, r#"{"name":"sample"}"#).expect("legacy manifest");
|
||||
|
||||
assert_eq!(find_plugin_manifest_path(&plugin_root), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_codex_claude_cursor_legacy_precedence() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user