bacow/claw-code
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Tighten plugin manifest validation and installed-plugin discovery

Browse Source 
Expanded the Rust plugin loader coverage around manifest parsing so invalid
permission values, invalid tool permissions, and multi-error manifests are
validated in a structured way. Added scan-path coverage for installed plugin
directories so both root and packaged manifests are discovered from the install
root, independent of registry entries.

Constraint: Keep plugin loader changes isolated to the plugins crate surface
Rejected: Add a new manifest crate for shared schemas | unnecessary scope for this pass
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: If manifest permissions or tool permission labels expand, update both the enums and validation tests together
Tested: cargo fmt --all; cargo test -p plugins
Not-tested: Cross-crate runtime consumption of any future expanded manifest permission variants
This commit is contained in:
Yeachan-Heo
2026-04-01 07:23:10 +00:00
parent a10bbaf8de
commit 28be7b3e24
1 changed files with 172 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

173
rust/crates/plugins/src/lib.rs
View File

@@ -2060,6 +2060,10 @@ mod tests {
}
fn write_tool_plugin(root: &Path, name: &str, version: &str) {
write_tool_plugin_with_name(root, name, version, "plugin_echo");
}
fn write_tool_plugin_with_name(root: &Path, name: &str, version: &str, tool_name: &str) {
let script_path = root.join("tools").join("echo-json.sh");
write_file(
&script_path,
@@ -2076,7 +2080,7 @@ mod tests {
write_file(
root.join(MANIFEST_RELATIVE_PATH).as_path(),
format!(
"{{\n \"name\": \"{name}\",\n \"version\": \"{version}\",\n \"description\": \"tool plugin\",\n \"tools\": [\n {{\n \"name\": \"plugin_echo\",\n \"description\": \"Echo JSON input\",\n \"inputSchema\": {{\"type\": \"object\", \"properties\": {{\"message\": {{\"type\": \"string\"}}}}, \"required\": [\"message\"], \"additionalProperties\": false}},\n \"command\": \"./tools/echo-json.sh\",\n \"requiredPermission\": \"workspace-write\"\n }}\n ]\n}}"
"{{\n \"name\": \"{name}\",\n \"version\": \"{version}\",\n \"description\": \"tool plugin\",\n \"tools\": [\n {{\n \"name\": \"{tool_name}\",\n \"description\": \"Echo JSON input\",\n \"inputSchema\": {{\"type\": \"object\", \"properties\": {{\"message\": {{\"type\": \"string\"}}}}, \"required\": [\"message\"], \"additionalProperties\": false}},\n \"command\": \"./tools/echo-json.sh\",\n \"requiredPermission\": \"workspace-write\"\n }}\n ]\n}}"
)
.as_str(),
);
@@ -2284,6 +2288,91 @@ mod tests {
let _ = fs::remove_dir_all(root);
}
#[test]
fn load_plugin_from_directory_rejects_invalid_tool_required_permission() {
let root = temp_dir("manifest-invalid-tool-permission");
write_file(
root.join("tools").join("echo.sh").as_path(),
"#!/bin/sh\ncat\n",
);
write_file(
root.join(MANIFEST_FILE_NAME).as_path(),
r#"{
"name": "invalid-tool-permission",
"version": "1.0.0",
"description": "Invalid tool permission validation",
"tools": [
{
"name": "echo_tool",
"description": "Echo tool",
"inputSchema": {"type": "object"},
"command": "./tools/echo.sh",
"requiredPermission": "admin"
}
]
}"#,
);
let error =
load_plugin_from_directory(&root).expect_err("invalid tool permission should fail");
match error {
PluginError::ManifestValidation(errors) => {
assert!(errors.iter().any(|error| matches!(
error,
PluginManifestValidationError::InvalidToolRequiredPermission {
tool_name,
permission
} if tool_name == "echo_tool" && permission == "admin"
)));
}
other => panic!("expected manifest validation errors, got {other}"),
}
let _ = fs::remove_dir_all(root);
}
#[test]
fn load_plugin_from_directory_accumulates_multiple_validation_errors() {
let root = temp_dir("manifest-multi-error");
write_file(
root.join(MANIFEST_FILE_NAME).as_path(),
r#"{
"name": "",
"version": "1.0.0",
"description": "",
"permissions": ["admin"],
"commands": [
{"name": "", "description": "", "command": "./commands/missing.sh"}
]
}"#,
);
let error =
load_plugin_from_directory(&root).expect_err("multiple manifest errors should fail");
match error {
PluginError::ManifestValidation(errors) => {
assert!(errors.len() >= 4);
assert!(errors.iter().any(|error| matches!(
error,
PluginManifestValidationError::EmptyField { field } if *field == "name"
)));
assert!(errors.iter().any(|error| matches!(
error,
PluginManifestValidationError::EmptyField { field }
if *field == "description"
)));
assert!(errors.iter().any(|error| matches!(
error,
PluginManifestValidationError::InvalidPermission { permission }
if permission == "admin"
)));
}
other => panic!("expected manifest validation errors, got {other}"),
}
let _ = fs::remove_dir_all(root);
}
#[test]
fn discovers_builtin_and_bundled_plugins() {
let manager = PluginManager::new(PluginManagerConfig::new(temp_dir("discover")));
@@ -2375,6 +2464,24 @@ mod tests {
let _ = fs::remove_dir_all(bundled_root);
}
#[test]
fn default_bundled_root_loads_repo_bundles_as_installed_plugins() {
let config_home = temp_dir("default-bundled-home");
let manager = PluginManager::new(PluginManagerConfig::new(&config_home));
let installed = manager
.list_installed_plugins()
.expect("default bundled plugins should auto-install");
assert!(installed
.iter()
.any(|plugin| plugin.metadata.id == "example-bundled@bundled"));
assert!(installed
.iter()
.any(|plugin| plugin.metadata.id == "sample-hooks@bundled"));
let _ = fs::remove_dir_all(config_home);
}
#[test]
fn persists_bundled_plugin_enable_state_across_reloads() {
let config_home = temp_dir("bundled-state-home");
@@ -2408,6 +2515,39 @@ mod tests {
let _ = fs::remove_dir_all(bundled_root);
}
#[test]
fn persists_bundled_plugin_disable_state_across_reloads() {
let config_home = temp_dir("bundled-disabled-home");
let bundled_root = temp_dir("bundled-disabled-root");
write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", true);
let mut config = PluginManagerConfig::new(&config_home);
config.bundled_root = Some(bundled_root.clone());
let mut manager = PluginManager::new(config);
manager
.disable("starter@bundled")
.expect("disable bundled plugin should succeed");
assert_eq!(
load_enabled_plugins(&manager.settings_path()).get("starter@bundled"),
Some(&false)
);
let mut reloaded_config = PluginManagerConfig::new(&config_home);
reloaded_config.bundled_root = Some(bundled_root.clone());
reloaded_config.enabled_plugins = load_enabled_plugins(&manager.settings_path());
let reloaded_manager = PluginManager::new(reloaded_config);
let reloaded = reloaded_manager
.list_installed_plugins()
.expect("bundled plugins should still be listed");
assert!(reloaded
.iter()
.any(|plugin| { plugin.metadata.id == "starter@bundled" && !plugin.enabled }));
let _ = fs::remove_dir_all(config_home);
let _ = fs::remove_dir_all(bundled_root);
}
#[test]
fn validates_plugin_source_before_install() {
let config_home = temp_dir("validate-home");
@@ -2552,4 +2692,35 @@ mod tests {
let _ = fs::remove_dir_all(config_home);
let _ = fs::remove_dir_all(bundled_root);
}
#[test]
fn list_installed_plugins_scans_packaged_manifests_in_install_root() {
let config_home = temp_dir("installed-packaged-scan-home");
let bundled_root = temp_dir("installed-packaged-scan-bundled");
let install_root = config_home.join("plugins").join("installed");
let installed_plugin_root = install_root.join("scan-packaged");
write_file(
installed_plugin_root.join(MANIFEST_RELATIVE_PATH).as_path(),
r#"{
"name": "scan-packaged",
"version": "1.0.0",
"description": "Packaged manifest in install root"
}"#,
);
let mut config = PluginManagerConfig::new(&config_home);
config.bundled_root = Some(bundled_root.clone());
config.install_root = Some(install_root);
let manager = PluginManager::new(config);
let installed = manager
.list_installed_plugins()
.expect("installed plugins should scan packaged manifests");
assert!(installed
.iter()
.any(|plugin| plugin.metadata.id == "scan-packaged@external"));
let _ = fs::remove_dir_all(config_home);
let _ = fs::remove_dir_all(bundled_root);
}
}