plugin: validate inspected tool schemas
This commit is contained in:
parent
dfa966dbfc
commit
982a1b75ed
|
|
@ -184,6 +184,7 @@ pub fn inspect_resolved_plugin_static(record: &ResolvedPluginRecord) -> PluginSt
|
||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
|
|
||||||
|
let duplicate_tool_names = duplicate_tool_names(record);
|
||||||
let tools = record
|
let tools = record
|
||||||
.manifest
|
.manifest
|
||||||
.tools
|
.tools
|
||||||
|
|
@ -192,9 +193,11 @@ pub fn inspect_resolved_plugin_static(record: &ResolvedPluginRecord) -> PluginSt
|
||||||
let permission = PluginPermission::tool(&tool.name);
|
let permission = PluginPermission::tool(&tool.name);
|
||||||
let requested = permission_requested(record, &permission);
|
let requested = permission_requested(record, &permission);
|
||||||
let granted = grant_allows(record, &permission);
|
let granted = grant_allows(record, &permission);
|
||||||
let diagnostic = authorize_plugin_tool(record, tool)
|
let mut diagnostics = validate_plugin_tool_definition(tool, &duplicate_tool_names);
|
||||||
.err()
|
if let Err(error) = authorize_plugin_tool(record, tool) {
|
||||||
.map(|error| error.bounded_message());
|
diagnostics.push(error.bounded_message());
|
||||||
|
}
|
||||||
|
let diagnostic = join_tool_diagnostics(diagnostics);
|
||||||
PluginToolEligibility {
|
PluginToolEligibility {
|
||||||
name: tool.name.clone(),
|
name: tool.name.clone(),
|
||||||
permission: permission.label(),
|
permission: permission.label(),
|
||||||
|
|
@ -230,6 +233,48 @@ fn grant_allows(record: &ResolvedPluginRecord, permission: &PluginPermission) ->
|
||||||
.any(|granted| granted == permission)
|
.any(|granted| granted == permission)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn duplicate_tool_names(record: &ResolvedPluginRecord) -> HashSet<String> {
|
||||||
|
let mut seen = HashSet::new();
|
||||||
|
let mut duplicates = HashSet::new();
|
||||||
|
for tool in &record.manifest.tools {
|
||||||
|
if !seen.insert(tool.name.clone()) {
|
||||||
|
duplicates.insert(tool.name.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
duplicates
|
||||||
|
}
|
||||||
|
|
||||||
|
fn validate_plugin_tool_definition(
|
||||||
|
tool: &PluginToolManifest,
|
||||||
|
duplicate_tool_names: &HashSet<String>,
|
||||||
|
) -> Vec<String> {
|
||||||
|
let mut diagnostics = Vec::new();
|
||||||
|
if duplicate_tool_names.contains(&tool.name) {
|
||||||
|
diagnostics.push(format!(
|
||||||
|
"tool `{}` has duplicate name within plugin manifest",
|
||||||
|
tool.name
|
||||||
|
));
|
||||||
|
}
|
||||||
|
if let Err(reason) = validate_tool_name(&tool.name) {
|
||||||
|
diagnostics.push(format!("tool `{}` has invalid name: {reason}", tool.name));
|
||||||
|
}
|
||||||
|
if let Err(reason) = validate_input_schema(&tool.input_schema) {
|
||||||
|
diagnostics.push(format!(
|
||||||
|
"tool `{}` has invalid input_schema: {reason}",
|
||||||
|
tool.name
|
||||||
|
));
|
||||||
|
}
|
||||||
|
diagnostics
|
||||||
|
}
|
||||||
|
|
||||||
|
fn join_tool_diagnostics(diagnostics: Vec<String>) -> Option<String> {
|
||||||
|
if diagnostics.is_empty() {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
Some(bounded_message(diagnostics.join("; ")))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
impl FeatureModule for PluginToolFeature {
|
impl FeatureModule for PluginToolFeature {
|
||||||
fn descriptor(&self) -> FeatureDescriptor {
|
fn descriptor(&self) -> FeatureDescriptor {
|
||||||
let mut descriptor =
|
let mut descriptor =
|
||||||
|
|
@ -1860,6 +1905,70 @@ input_schema = { type = "object", additionalProperties = true }
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn static_inspection_reports_invalid_tool_definition() {
|
||||||
|
let mut bad_schema = tool("Echo");
|
||||||
|
bad_schema.input_schema = json!({"type":"string"});
|
||||||
|
let mut record = record(vec![bad_schema]);
|
||||||
|
record.manifest.runtime = Some(PluginRuntimeManifest {
|
||||||
|
kind: "wasm".to_string(),
|
||||||
|
entry: "plugin.wasm".to_string(),
|
||||||
|
abi: Some("yoi-plugin-wasm-1".to_string()),
|
||||||
|
});
|
||||||
|
|
||||||
|
let inspection = inspect_resolved_plugin_static(&record);
|
||||||
|
|
||||||
|
assert!(!inspection.statically_eligible());
|
||||||
|
assert!(!inspection.tools[0].eligible);
|
||||||
|
let diagnostic = inspection.tools[0]
|
||||||
|
.diagnostic
|
||||||
|
.as_deref()
|
||||||
|
.unwrap_or_default();
|
||||||
|
assert!(diagnostic.contains("invalid input_schema"));
|
||||||
|
assert!(diagnostic.contains("root schema type must be `object`"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn static_inspection_reports_invalid_and_duplicate_tool_names() {
|
||||||
|
let mut invalid = tool("Bad Tool");
|
||||||
|
invalid.input_schema = json!({"type":"object"});
|
||||||
|
let mut first_duplicate = tool("Echo");
|
||||||
|
let mut second_duplicate = tool("Echo");
|
||||||
|
first_duplicate.input_schema = json!({"type":"object"});
|
||||||
|
second_duplicate.input_schema = json!({"type":"object"});
|
||||||
|
let mut record = record(vec![invalid, first_duplicate, second_duplicate]);
|
||||||
|
record.manifest.runtime = Some(PluginRuntimeManifest {
|
||||||
|
kind: "wasm".to_string(),
|
||||||
|
entry: "plugin.wasm".to_string(),
|
||||||
|
abi: Some("yoi-plugin-wasm-1".to_string()),
|
||||||
|
});
|
||||||
|
|
||||||
|
let inspection = inspect_resolved_plugin_static(&record);
|
||||||
|
|
||||||
|
assert!(!inspection.statically_eligible());
|
||||||
|
assert!(
|
||||||
|
inspection.tools[0]
|
||||||
|
.diagnostic
|
||||||
|
.as_deref()
|
||||||
|
.unwrap_or_default()
|
||||||
|
.contains("invalid name")
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
inspection.tools[1]
|
||||||
|
.diagnostic
|
||||||
|
.as_deref()
|
||||||
|
.unwrap_or_default()
|
||||||
|
.contains("duplicate name")
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
inspection.tools[2]
|
||||||
|
.diagnostic
|
||||||
|
.as_deref()
|
||||||
|
.unwrap_or_default()
|
||||||
|
.contains("duplicate name")
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
fn write_stored_zip(path: &Path, files: &[(&str, &[u8])]) {
|
fn write_stored_zip(path: &Path, files: &[(&str, &[u8])]) {
|
||||||
let mut out = Vec::new();
|
let mut out = Vec::new();
|
||||||
let mut central = Vec::new();
|
let mut central = Vec::new();
|
||||||
|
|
|
||||||
|
|
@ -989,6 +989,64 @@ mod tests {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn invalid_tool_schema_and_name_are_rejected_in_json_and_human_output() {
|
||||||
|
let dir = tempdir().unwrap();
|
||||||
|
let workspace = dir.path();
|
||||||
|
let bad_schema_manifest = plugin_manifest("bad_schema", "Echo", "string", &["Echo"]);
|
||||||
|
let bad_name_manifest = plugin_manifest("bad_name", "Bad Tool", "object", &["Bad Tool"]);
|
||||||
|
let bad_schema_digest =
|
||||||
|
write_plugin_manifest(workspace, "bad_schema", &bad_schema_manifest);
|
||||||
|
let bad_name_digest = write_plugin_manifest(workspace, "bad_name", &bad_name_manifest);
|
||||||
|
let mut config = PluginConfig::default();
|
||||||
|
config.enabled.push(enablement(
|
||||||
|
"project:bad_schema",
|
||||||
|
"0.1.0",
|
||||||
|
bad_schema_digest,
|
||||||
|
&["Echo"],
|
||||||
|
));
|
||||||
|
config.enabled.push(enablement(
|
||||||
|
"project:bad_name",
|
||||||
|
"0.1.0",
|
||||||
|
bad_name_digest,
|
||||||
|
&["Bad Tool"],
|
||||||
|
));
|
||||||
|
|
||||||
|
let snapshot = inspect_snapshot(workspace, &config);
|
||||||
|
let bad_schema = select_item(&snapshot, "project:bad_schema").unwrap();
|
||||||
|
let bad_name = select_item(&snapshot, "project:bad_name").unwrap();
|
||||||
|
|
||||||
|
assert_eq!(bad_schema.status, "rejected");
|
||||||
|
assert_eq!(bad_name.status, "rejected");
|
||||||
|
assert!(!bad_schema.tools[0].eligible);
|
||||||
|
assert!(!bad_name.tools[0].eligible);
|
||||||
|
|
||||||
|
let list_json = serde_json::to_value(&snapshot).unwrap();
|
||||||
|
assert!(list_json["items"].as_array().unwrap().iter().any(|item| {
|
||||||
|
item["reference"] == "project:bad_schema"
|
||||||
|
&& item["status"] == "rejected"
|
||||||
|
&& item["tools"][0]["diagnostic"]
|
||||||
|
.as_str()
|
||||||
|
.unwrap_or_default()
|
||||||
|
.contains("invalid input_schema")
|
||||||
|
}));
|
||||||
|
let show_json = serde_json::to_value(bad_name).unwrap();
|
||||||
|
assert_eq!(show_json["status"], "rejected");
|
||||||
|
assert!(
|
||||||
|
show_json["tools"][0]["diagnostic"]
|
||||||
|
.as_str()
|
||||||
|
.unwrap_or_default()
|
||||||
|
.contains("invalid name")
|
||||||
|
);
|
||||||
|
|
||||||
|
let list_output = render_list_snapshot_human(&snapshot).unwrap();
|
||||||
|
assert!(list_output.contains("project:bad_schema [rejected]"));
|
||||||
|
assert!(list_output.contains("project:bad_name [rejected]"));
|
||||||
|
let show_output = render_item_human(bad_schema).unwrap();
|
||||||
|
assert!(show_output.contains("invalid input_schema"));
|
||||||
|
assert!(show_output.contains("eligible=false"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn ambiguous_ref_is_bounded_error() {
|
fn ambiguous_ref_is_bounded_error() {
|
||||||
let snapshot = PluginInspectionSnapshot {
|
let snapshot = PluginInspectionSnapshot {
|
||||||
|
|
@ -1042,6 +1100,66 @@ mod tests {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn enablement(
|
||||||
|
id: &str,
|
||||||
|
version: &str,
|
||||||
|
digest: String,
|
||||||
|
tool_permissions: &[&str],
|
||||||
|
) -> PluginEnablementConfig {
|
||||||
|
let mut permissions = vec![PluginPermission::surface(PluginSurface::Tool)];
|
||||||
|
permissions.extend(
|
||||||
|
tool_permissions
|
||||||
|
.iter()
|
||||||
|
.map(|tool_name| PluginPermission::tool(*tool_name)),
|
||||||
|
);
|
||||||
|
PluginEnablementConfig {
|
||||||
|
id: id.to_string(),
|
||||||
|
digest: Some(digest.clone()),
|
||||||
|
version: Some(PluginExactVersion(version.to_string())),
|
||||||
|
surfaces: vec![PluginSurface::Tool],
|
||||||
|
grants: PluginGrantConfig {
|
||||||
|
id: Some(id.to_string()),
|
||||||
|
version: Some(PluginExactVersion(version.to_string())),
|
||||||
|
digest: Some(digest),
|
||||||
|
permissions,
|
||||||
|
},
|
||||||
|
config: None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn plugin_manifest(
|
||||||
|
id: &str,
|
||||||
|
tool_name: &str,
|
||||||
|
schema_type: &str,
|
||||||
|
permission_tools: &[&str],
|
||||||
|
) -> String {
|
||||||
|
let permissions = permission_tools
|
||||||
|
.iter()
|
||||||
|
.map(|tool| format!(r#"{{ kind = "tool", name = "{tool}" }}"#))
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join(", ");
|
||||||
|
format!(
|
||||||
|
r#"
|
||||||
|
schema_version = 1
|
||||||
|
id = "{id}"
|
||||||
|
name = "{id}"
|
||||||
|
version = "0.1.0"
|
||||||
|
surfaces = ["tool"]
|
||||||
|
permissions = [{{ kind = "surface", surface = "tool" }}, {permissions}]
|
||||||
|
|
||||||
|
[runtime]
|
||||||
|
kind = "wasm"
|
||||||
|
entry = "plugin.wasm"
|
||||||
|
abi = "yoi-plugin-wasm-1"
|
||||||
|
|
||||||
|
[[tools]]
|
||||||
|
name = "{tool_name}"
|
||||||
|
description = "Test tool"
|
||||||
|
input_schema = {{ type = "{schema_type}" }}
|
||||||
|
"#
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
fn write_plugin_package(workspace: &Path, id: &str) -> String {
|
fn write_plugin_package(workspace: &Path, id: &str) -> String {
|
||||||
let manifest = format!(
|
let manifest = format!(
|
||||||
r#"
|
r#"
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user