forked from 1b-agent/opencadstudio-upstream
feat(plugin): wrap external-plugin calls in a panic guard
A buggy external plugin that panicked anywhere — during load (version, register, manifest), during ribbon build / dispatch, or inside an interactive command (prompt, on_point, on_enter, on_object_pick) — took the host down with it (#134). Run every cdylib entry point under catch_unwind so the host stays up: the plugin's call is treated as inert (load fails, dispatch returns "didn't handle it", interactive hooks resolve to Cancel) and a "Plugin <hook> panicked: …" line lands in the command line on the next tick. (#145) Scope is Rust panics across the cdylib boundary, not genuine UB — real isolation (segfault-proof) still requires out-of-process plugins. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
2a260be31d
commit
f860fb39c2
5 files changed
+122
-14
No files matched your search
+21
-7
@@ -226,23 +226,37 @@ impl crate::command::CadCommand for PluginInteractiveAdapter {
|
||||
fn name(&self) -> &'static str {
|
||||
"PLUGIN"
|
||||
}
|
||||
// Every call into the plugin runs under a panic guard (#145): a buggy plugin
|
||||
// that panics mid-command leaves the host running — the command just ends.
|
||||
fn prompt(&self) -> String {
|
||||
self.inner.prompt()
|
||||
crate::plugin::guard("InteractiveCommand::prompt", || self.inner.prompt())
|
||||
.unwrap_or_default()
|
||||
}
|
||||
fn on_point(&mut self, pt: glam::Vec3) -> crate::command::CmdResult {
|
||||
plugin_step_to_result(self.inner.on_point([pt.x as f64, pt.y as f64, pt.z as f64]))
|
||||
crate::plugin::guard("InteractiveCommand::on_point", || {
|
||||
self.inner.on_point([pt.x as f64, pt.y as f64, pt.z as f64])
|
||||
})
|
||||
.map(plugin_step_to_result)
|
||||
.unwrap_or(crate::command::CmdResult::Cancel)
|
||||
}
|
||||
fn on_enter(&mut self) -> crate::command::CmdResult {
|
||||
plugin_step_to_result(self.inner.on_enter())
|
||||
crate::plugin::guard("InteractiveCommand::on_enter", || self.inner.on_enter())
|
||||
.map(plugin_step_to_result)
|
||||
.unwrap_or(crate::command::CmdResult::Cancel)
|
||||
}
|
||||
fn needs_entity_pick(&self) -> bool {
|
||||
self.inner.needs_object_pick()
|
||||
crate::plugin::guard("InteractiveCommand::needs_object_pick", || {
|
||||
self.inner.needs_object_pick()
|
||||
})
|
||||
.unwrap_or(false)
|
||||
}
|
||||
fn on_entity_pick(&mut self, handle: Handle, pt: glam::Vec3) -> crate::command::CmdResult {
|
||||
plugin_step_to_result(
|
||||
crate::plugin::guard("InteractiveCommand::on_object_pick", || {
|
||||
self.inner
|
||||
.on_object_pick(handle, [pt.x as f64, pt.y as f64, pt.z as f64]),
|
||||
)
|
||||
.on_object_pick(handle, [pt.x as f64, pt.y as f64, pt.z as f64])
|
||||
})
|
||||
.map(plugin_step_to_result)
|
||||
.unwrap_or(crate::command::CmdResult::Cancel)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -414,6 +414,14 @@ impl OpenCADStudio {
|
||||
}
|
||||
}
|
||||
|
||||
// Surface any plugin-guard panic messages that piled up since
|
||||
// the last tick (the host singleton isn't reachable from inside
|
||||
// the plugin hooks, so they queue and we flush here). (#145)
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
for msg in crate::plugin::drain_errors() {
|
||||
self.command_line.push_error(&msg);
|
||||
}
|
||||
|
||||
Task::none()
|
||||
}
|
||||
|
||||
|
||||
+10
-3
@@ -311,7 +311,10 @@ mod loader {
|
||||
let version: libloading::Symbol<extern "C" fn() -> u32> = lib
|
||||
.get(b"ocs_plugin_api_version")
|
||||
.map_err(|_| "missing ocs_plugin_api_version symbol".to_string())?;
|
||||
let v = version();
|
||||
// Wrap the version + register calls in a panic guard so a plugin
|
||||
// that panics during load can't take down the host. (#145)
|
||||
let v = crate::plugin::guard("version", || version())
|
||||
.ok_or_else(|| "ocs_plugin_api_version panicked".to_string())?;
|
||||
if v != ocs_plugin_api::API_VERSION {
|
||||
return Err(format!(
|
||||
"API version {v} != host {}",
|
||||
@@ -324,12 +327,16 @@ mod loader {
|
||||
> = lib
|
||||
.get(b"ocs_plugin_register")
|
||||
.map_err(|_| "missing ocs_plugin_register symbol".to_string())?;
|
||||
let raw = register();
|
||||
let raw = crate::plugin::guard("register", || register())
|
||||
.ok_or_else(|| "ocs_plugin_register panicked".to_string())?;
|
||||
if raw.is_null() {
|
||||
return Err("ocs_plugin_register returned null".into());
|
||||
}
|
||||
let plugin = *Box::from_raw(raw);
|
||||
let id = plugin.manifest().id.to_string();
|
||||
// The manifest read happens once at load; guard it too — a buggy
|
||||
// manifest() that panics here would otherwise crash startup. (#145)
|
||||
let id = crate::plugin::guard("manifest", || plugin.manifest().id.to_string())
|
||||
.ok_or_else(|| "plugin manifest() panicked".to_string())?;
|
||||
Ok(LoadedPlugin {
|
||||
plugin,
|
||||
_lib: lib,
|
||||
|
||||
+69
-1
@@ -8,4 +8,72 @@ pub mod marketplace;
|
||||
pub mod registry;
|
||||
|
||||
pub use registry::{all_ribbon_modules, ribbon_modules_enabled};
|
||||
pub(crate) use registry::try_dispatch;
|
||||
pub(crate) use registry::try_dispatch;
|
||||
|
||||
/// Run a plugin entry point under a panic guard so a buggy external plugin
|
||||
/// can't take down the host. Returns `None` (after logging) when the plugin
|
||||
/// panicked; the caller substitutes a safe default. Catches Rust panics across
|
||||
/// the cdylib boundary — it does NOT contain genuine UB (a plugin that
|
||||
/// segfaults or violates memory safety via `unsafe` can still crash the host;
|
||||
/// only out-of-process isolation would). (#145)
|
||||
pub(crate) fn guard<T>(what: &str, f: impl FnOnce() -> T) -> Option<T> {
|
||||
match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) {
|
||||
Ok(v) => Some(v),
|
||||
Err(payload) => {
|
||||
let msg = payload
|
||||
.downcast_ref::<&str>()
|
||||
.map(|s| (*s).to_string())
|
||||
.or_else(|| payload.downcast_ref::<String>().cloned())
|
||||
.unwrap_or_else(|| "panic".to_string());
|
||||
let line = format!("Plugin {what} panicked: {msg} — call ignored");
|
||||
eprintln!("[plugin] {line}");
|
||||
// The plugin hooks run deep inside the update / render loop where
|
||||
// the app's command line isn't reachable, so queue the error and
|
||||
// let the next update tick drain it into the command line. (#145)
|
||||
errors().lock().unwrap().push(line);
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Process-wide queue of plugin-guard errors waiting to surface in the host's
|
||||
/// command-line history. Flushed by [`drain_errors`] from the app loop.
|
||||
fn errors() -> &'static std::sync::Mutex<Vec<String>> {
|
||||
static E: std::sync::OnceLock<std::sync::Mutex<Vec<String>>> = std::sync::OnceLock::new();
|
||||
E.get_or_init(|| std::sync::Mutex::new(Vec::new()))
|
||||
}
|
||||
|
||||
/// Take and return every queued plugin-guard error since the last drain.
|
||||
pub(crate) fn drain_errors() -> Vec<String> {
|
||||
std::mem::take(&mut *errors().lock().unwrap())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::guard;
|
||||
|
||||
#[test]
|
||||
fn returns_value_on_success() {
|
||||
let r = guard("ok", || 42);
|
||||
assert_eq!(r, Some(42));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn swallows_panic_and_queues_error_message() {
|
||||
let _ = super::drain_errors(); // clear anything from earlier tests
|
||||
// Silence the default panic-hook stderr noise during the test (the
|
||||
// guard's own eprintln stays — that's the diagnostic we want).
|
||||
let prev = std::panic::take_hook();
|
||||
std::panic::set_hook(Box::new(|_| {}));
|
||||
let r: Option<i32> = guard("boom", || panic!("plugin went wrong"));
|
||||
std::panic::set_hook(prev);
|
||||
assert_eq!(r, None);
|
||||
// The user-visible message lands in the plugin error queue (the app
|
||||
// loop drains it into the command line on the next tick).
|
||||
let queued = super::drain_errors();
|
||||
assert!(
|
||||
queued.iter().any(|m| m.contains("boom") && m.contains("plugin went wrong")),
|
||||
"expected an error queued for the command line, got: {queued:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
+14
-3
@@ -23,8 +23,15 @@ pub fn ribbon_modules_enabled(
|
||||
let mut addons: Vec<(i32, Box<dyn CadModule>)> = Vec::new();
|
||||
crate::plugin::external::with_loaded(|loaded| {
|
||||
for lp in loaded {
|
||||
if !disabled.contains(lp.id.as_str()) {
|
||||
addons.push((lp.plugin().manifest().ribbon_order, lp.plugin().ribbon()));
|
||||
if disabled.contains(lp.id.as_str()) {
|
||||
continue;
|
||||
}
|
||||
// Guard the plugin's ribbon build so a panic there can't take
|
||||
// down the host — the plugin just contributes no tab. (#145)
|
||||
if let Some(entry) = crate::plugin::guard("ribbon", || {
|
||||
(lp.plugin().manifest().ribbon_order, lp.plugin().ribbon())
|
||||
}) {
|
||||
addons.push(entry);
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -48,7 +55,11 @@ pub(crate) fn try_dispatch(app: &mut OpenCADStudio, tab: usize, cmd: &str) -> bo
|
||||
if disabled.contains(lp.id.as_str()) {
|
||||
continue;
|
||||
}
|
||||
if lp.plugin().dispatch(&mut host, cmd) {
|
||||
// A panic inside the plugin's dispatch must not crash the host;
|
||||
// treat a panicking plugin as "didn't handle it". (#145)
|
||||
if crate::plugin::guard("dispatch", || lp.plugin().dispatch(&mut host, cmd))
|
||||
.unwrap_or(false)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user