Merge pull request #447 from bl4vk-0bsidi4n/fix/harden-plugin-system-220

fix: harden plugin loading mechanics with type-safe paths and panic boundaries #220

Author: Nanle-code

src/plugins/loader.rs

@@ -37,6 +37,8 @@ pub enum PluginLoadError {
     },
     /// The `starforge-plugin.toml` manifest failed validation.
     ManifestIncompatible { path: String, detail: String },
+    /// The plugin panicked or crashed unexpectedly during active registration routines.
+    RegistrationRuntimePanic { path: String, detail: String },
 }
 
 impl PluginLoadError {
@@ -48,6 +50,7 @@ impl PluginLoadError {
             Self::AbiBuildMismatch { .. } => "abi_mismatch",
             Self::UnsupportedCoreVersion { .. } => "unsupported_core_version",
             Self::ManifestIncompatible { .. } => "manifest_incompatible",
+            Self::RegistrationRuntimePanic { .. } => "runtime_panic",
         }
     }
 
@@ -85,6 +88,11 @@ impl PluginLoadError {
                  Detail: {detail}\n  \
                  Fix: Update 'starforge-plugin.toml' to match the running StarForge version.",
             ),
+            Self::RegistrationRuntimePanic { path, detail } => format!(
+                "Plugin crashed during registration for '{path}'.\n  \
+                 Detail: {detail}\n  \
+                 Fix: Review third-party plugin internal setup safety rules or contact the maintainer.",
+            ),
         }
     }
 }
@@ -120,7 +128,7 @@ impl PluginManager {
     /// # Safety
     /// The caller must ensure the plugin at `path` is a valid StarForge plugin
     /// compiled with a compatible Rust toolchain and ABI.
-    pub unsafe fn load_plugin<P: AsRef<OsStr>>(&mut self, path: P) -> Result<()> {
+    pub unsafe fn load_plugin<P: AsRef<Path>>(&mut self, path: P) -> Result<()> {
         self.load_plugin_diagnosed(path)
             .map_err(|e| anyhow::anyhow!("{}", e))
     }
@@ -130,15 +138,15 @@ impl PluginManager {
     ///
     /// # Safety
     /// Same contract as [`load_plugin`].
-    pub unsafe fn load_plugin_diagnosed<P: AsRef<OsStr>>(
+    pub unsafe fn load_plugin_diagnosed<P: AsRef<Path>>(
         &mut self,
         path: P,
     ) -> std::result::Result<(), PluginLoadError> {
         let path_ref = path.as_ref();
         let path_display = path_ref.to_string_lossy().to_string();
 
         // ── Open the shared library ──────────────────────────────────────────
-        let library = Library::new(path_ref).map_err(|e| PluginLoadError::InvalidLibrary {
+        let library = Library::new(path_ref.as_os_str()).map_err(|e| PluginLoadError::InvalidLibrary {
             path: path_display.clone(),
             detail: e.to_string(),
         })?;
@@ -174,7 +182,7 @@ impl PluginManager {
         }
 
         // ── Manifest compatibility (if present beside the library) ───────────
-        if let Ok(Some(mf)) = manifest::load_manifest_for_library(Path::new(path_ref)) {
+        if let Ok(Some(mf)) = manifest::load_manifest_for_library(path_ref) {
             mf.validate()
                 .map_err(|e| PluginLoadError::ManifestIncompatible {
                     path: path_display.clone(),
@@ -183,7 +191,25 @@ impl PluginManager {
         }
 
         let mut registrar = ProxyRegistrar::new();
-        (decl.register)(&mut registrar);
+        
+        // Protect the system execution loop from third-party registration panics
+        let register_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            (decl.register)(&mut registrar);
+        }));
+
+        if let Err(panic_payload) = register_result {
+            let detail = if let Some(s) = panic_payload.downcast_ref::<&str>() {
+                s.to_string()
+            } else if let Some(s) = panic_payload.downcast_ref::<String>() {
+                s.clone()
+            } else {
+                "Unknown closure panic origin".to_string()
+            };
+            return Err(PluginLoadError::RegistrationRuntimePanic {
+                path: path_display,
+                detail,
+            });
+        }
 
         let plugin_core_version = decl.core_version.to_string();
         for plugin in registrar.plugins {
@@ -218,7 +244,7 @@ impl PluginManager {
         if let Some((plugin, _)) = self.plugins.get(name) {
             plugin.execute(args)
         } else {
-            Err(format!("Plugin '{}' not found", name))
+            return Err(format!("Plugin '{}' not found", name));
         }
     }
 }
@@ -294,6 +320,15 @@ mod tests {
         assert_eq!(e.category(), "manifest_incompatible");
     }
 
+    #[test]
+    fn registration_runtime_panic_category() {
+        let e = PluginLoadError::RegistrationRuntimePanic {
+            path: "/tmp/plugin.so".into(),
+            detail: "poisoned pointer access".into(),
+        };
+        assert_eq!(e.category(), "runtime_panic");
+    }
+
     // ── Diagnostic messages contain actionable guidance ──────────────────────
 
     #[test]
@@ -357,6 +392,17 @@ mod tests {
         assert!(msg.contains("0.1.0"));
     }
 
+    #[test]
+    fn registration_runtime_panic_diagnostic_mentions_rules() {
+        let e = PluginLoadError::RegistrationRuntimePanic {
+            path: "/tmp/plugin.so".into(),
+            detail: "forced assertion failure".into(),
+        };
+        let msg = e.diagnostic();
+        assert!(msg.contains("crashed during registration"));
+        assert!(msg.contains("forced assertion failure"));
+    }
+
     #[test]
     fn display_matches_diagnostic() {
         let e = PluginLoadError::InvalidLibrary {
@@ -371,12 +417,12 @@ mod tests {
     #[test]
     fn nonexistent_path_returns_invalid_library() {
         let mut pm = PluginManager::new();
-        let result = unsafe { pm.load_plugin_diagnosed("/nonexistent/path/plugin.so") };
+        let result = unsafe { pm.load_plugin_diagnosed(Path::new("/nonexistent/path/plugin.so")) };
         match result {
             Err(PluginLoadError::InvalidLibrary { path, .. }) => {
                 assert!(path.contains("plugin.so"));
             }
             other => panic!("Expected InvalidLibrary, got {:?}", other),
         }
     }
-}
+}