From 0e4d1a9b932209ede7102675db374c000acdc785 Mon Sep 17 00:00:00 2001 From: MerhiOPS Date: Mon, 16 Mar 2026 12:44:04 +0200 Subject: [PATCH 1/4] fix high severity runtime and dev IPC issues --- .../volt-runner/src/ipc_bridge/worker_pool.rs | 103 ++++++- crates/volt-runner/src/js_runtime.rs | 39 ++- crates/volt-runner/src/js_runtime/ipc.rs | 36 +-- crates/volt-runner/src/js_runtime/requests.rs | 3 +- .../src/js_runtime/serde_support.rs | 48 ++- .../js_runtime/tests/native_ipc/roundtrip.rs | 66 +++- crates/volt-runner/src/js_runtime/worker.rs | 2 + .../src/plugin_manager/lifecycle_bus.rs | 20 +- .../plugin_manager/process/child/messaging.rs | 15 +- .../src/plugin_manager/process/io.rs | 33 +- .../src/plugin_manager/tests/lifecycle_bus.rs | 46 +++ .../dev-backend-runtime-modules.test.ts | 288 ++++++++++++++++++ .../src/__tests__/dev-backend-test-utils.ts | 22 ++ .../src/__tests__/dev-backend.test.ts | 100 +++--- packages/volt-cli/src/commands/dev.ts | 1 + packages/volt-cli/src/commands/dev/backend.ts | 4 + .../commands/dev/runtime-modules/shared.ts | 21 ++ .../dev/runtime-modules/volt-clipboard.ts | 4 +- .../commands/dev/runtime-modules/volt-http.ts | 93 +++++- .../dev/runtime-modules/volt-plugins.ts | 98 ++++++ .../runtime-modules/volt-secure-storage.ts | 7 +- .../dev/runtime-modules/volt-shell.ts | 4 +- 22 files changed, 941 insertions(+), 112 deletions(-) create mode 100644 packages/volt-cli/src/__tests__/dev-backend-runtime-modules.test.ts create mode 100644 packages/volt-cli/src/__tests__/dev-backend-test-utils.ts create mode 100644 packages/volt-cli/src/commands/dev/runtime-modules/volt-plugins.ts diff --git a/crates/volt-runner/src/ipc_bridge/worker_pool.rs b/crates/volt-runner/src/ipc_bridge/worker_pool.rs index 739019e..2fd805d 100644 --- a/crates/volt-runner/src/ipc_bridge/worker_pool.rs +++ b/crates/volt-runner/src/ipc_bridge/worker_pool.rs @@ -1,8 +1,10 @@ +use std::panic::{self, AssertUnwindSafe}; use std::sync::Mutex; use std::thread::{self, JoinHandle}; use std::time::Duration; use crossbeam_channel as channel; +use volt_core::ipc::{IPC_HANDLER_ERROR_CODE, IpcResponse}; use super::dispatch::dispatch_ipc_task; use super::in_flight::InFlightTracker; @@ -45,16 +47,15 @@ impl IpcWorkerPool { Err(_) => return, }; - let response = dispatch_ipc_task( - &worker_runtime_client, - worker_plugin_manager.as_ref(), - &task.raw, - &task.request_id, - task.timeout, - ); - - send_response_to_window(&task.js_window_id, response); - worker_tracker.release(&task.js_window_id); + process_task(&task, &worker_tracker, || { + dispatch_ipc_task( + &worker_runtime_client, + worker_plugin_manager.as_ref(), + &task.raw, + &task.request_id, + task.timeout, + ) + }); } }); @@ -97,3 +98,85 @@ impl IpcWorkerPool { } } } + +struct InFlightSlotGuard<'a> { + tracker: &'a InFlightTracker, + js_window_id: &'a str, +} + +impl<'a> InFlightSlotGuard<'a> { + fn new(tracker: &'a InFlightTracker, js_window_id: &'a str) -> Self { + Self { + tracker, + js_window_id, + } + } +} + +impl Drop for InFlightSlotGuard<'_> { + fn drop(&mut self) { + self.tracker.release(self.js_window_id); + } +} + +fn process_task( + task: &IpcDispatchTask, + tracker: &InFlightTracker, + dispatch: impl FnOnce() -> IpcResponse, +) { + let _slot_guard = InFlightSlotGuard::new(tracker, &task.js_window_id); + let response = match panic::catch_unwind(AssertUnwindSafe(dispatch)) { + Ok(response) => response, + Err(payload) => { + let panic_message = panic_payload_message(&payload); + tracing::error!( + request_id = %task.request_id, + js_window_id = %task.js_window_id, + panic = %panic_message, + "IPC bridge worker recovered from a panicking task" + ); + IpcResponse::error_with_code( + task.request_id.clone(), + format!("IPC bridge worker panicked while handling the request: {panic_message}"), + IPC_HANDLER_ERROR_CODE.to_string(), + ) + } + }; + + send_response_to_window(&task.js_window_id, response); +} + +fn panic_payload_message(payload: &Box) -> String { + if let Some(message) = payload.downcast_ref::<&str>() { + return (*message).to_string(); + } + if let Some(message) = payload.downcast_ref::() { + return message.clone(); + } + "unknown panic payload".to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn process_task_releases_window_slot_when_dispatch_panics() { + let tracker = InFlightTracker::new(1, 1); + assert!(tracker.try_acquire("window-1")); + + process_task( + &IpcDispatchTask { + js_window_id: "window-1".to_string(), + raw: "{}".to_string(), + request_id: "req-1".to_string(), + timeout: Duration::from_millis(5), + }, + &tracker, + || panic!("boom"), + ); + + assert_eq!(tracker.in_flight_for("window-1"), 0); + assert!(tracker.try_acquire("window-1")); + } +} diff --git a/crates/volt-runner/src/js_runtime.rs b/crates/volt-runner/src/js_runtime.rs index 5010327..3e008cc 100644 --- a/crates/volt-runner/src/js_runtime.rs +++ b/crates/volt-runner/src/js_runtime.rs @@ -1,10 +1,10 @@ use std::path::PathBuf; use std::sync::mpsc::{self, RecvTimeoutError, Sender}; use std::thread::{self, JoinHandle}; -use std::time::Duration; +use std::time::{Duration, Instant}; use serde_json::Value as JsonValue; -use volt_core::ipc::{IPC_HANDLER_TIMEOUT_CODE, IpcResponse}; +use volt_core::ipc::IpcResponse; use crate::plugin_manager::PluginManager; @@ -178,11 +178,14 @@ impl JsRuntimeClient { timeout: Duration, ) -> Result { let response_timeout = normalize_ipc_timeout(timeout); + let wait_started_at = Instant::now(); + let deadline = Instant::now() + response_timeout; let (response_tx, response_rx) = mpsc::channel(); self.request_tx .send(RuntimeRequest::DispatchIpc { raw: raw.to_string(), timeout: response_timeout, + deadline, response_tx, }) .map_err(|_| "js runtime worker is not running".to_string())?; @@ -191,17 +194,11 @@ impl JsRuntimeClient { Ok(response) => Ok(response), Err(RecvTimeoutError::Timeout) => { let method = serde_support::extract_ipc_method(raw); - Ok(IpcResponse::error_with_details( + Ok(serde_support::ipc_timeout_response( serde_support::extract_ipc_request_id(raw), - format!( - "IPC handler '{method}' timed out after {}ms before the runtime returned a response", - response_timeout.as_millis() - ), - IPC_HANDLER_TIMEOUT_CODE.to_string(), - serde_json::json!({ - "timeoutMs": response_timeout.as_millis(), - "method": method - }), + method, + response_timeout, + wait_started_at.elapsed().min(response_timeout), )) } Err(RecvTimeoutError::Disconnected) => { @@ -268,3 +265,21 @@ pub(crate) fn normalize_ipc_timeout(timeout: Duration) -> Duration { timeout } } + +pub(crate) fn ipc_inner_timeout_budget(timeout: Duration) -> Duration { + let timeout_ms = timeout.as_millis(); + let margin = if timeout_ms > 1_000 { + Duration::from_millis(1_000) + } else if timeout_ms > 10 { + Duration::from_millis(10) + } else if timeout_ms > 1 { + Duration::from_millis(1) + } else { + Duration::ZERO + }; + + timeout + .checked_sub(margin) + .filter(|budget| !budget.is_zero()) + .unwrap_or(timeout) +} diff --git a/crates/volt-runner/src/js_runtime/ipc.rs b/crates/volt-runner/src/js_runtime/ipc.rs index 63a3519..b85cb9d 100644 --- a/crates/volt-runner/src/js_runtime/ipc.rs +++ b/crates/volt-runner/src/js_runtime/ipc.rs @@ -7,10 +7,7 @@ use boa_engine::job::SimpleJobExecutor; use boa_engine::object::builtins::JsPromise; use boa_engine::{Context, JsValue, js_string}; use serde_json::Value as JsonValue; -use volt_core::ipc::{ - IPC_HANDLER_ERROR_CODE, IPC_HANDLER_TIMEOUT_CODE, IPC_MAX_REQUEST_BYTES, IpcRequest, - IpcResponse, -}; +use volt_core::ipc::{IPC_HANDLER_ERROR_CODE, IPC_MAX_REQUEST_BYTES, IpcRequest, IpcResponse}; use super::IPC_DISPATCH_SAFE_GLOBAL; const IPC_PROTOTYPE_CHECK_MAX_DEPTH: usize = 64; @@ -48,6 +45,7 @@ pub(super) async fn dispatch_ipc_request( ipc_state: &mut IpcRuntimeState, raw: &str, timeout: Duration, + deadline: Instant, ) -> IpcResponse { let request = match parse_ipc_request(raw, ipc_state) { Ok(request) => request, @@ -68,10 +66,21 @@ pub(super) async fn dispatch_ipc_request( } let dispatch_timeout = super::normalize_ipc_timeout(timeout); + let remaining_timeout = deadline.saturating_duration_since(Instant::now()); + let queue_delay = dispatch_timeout.saturating_sub(remaining_timeout); + if remaining_timeout.is_zero() { + return super::serde_support::ipc_timeout_response( + request.id, + request.method, + dispatch_timeout, + queue_delay, + ); + } + let handler_timeout = super::ipc_inner_timeout_budget(remaining_timeout); let request_id = request.id.clone(); let dispatch_result = tokio::time::timeout( - dispatch_timeout, + handler_timeout, dispatch_ipc_handler(context, job_executor, &request.method, &request.args), ) .await; @@ -83,18 +92,11 @@ pub(super) async fn dispatch_ipc_request( Ok(Err(message)) => { IpcResponse::error_with_code(request_id, message, IPC_HANDLER_ERROR_CODE.to_string()) } - Err(_) => IpcResponse::error_with_details( + Err(_) => super::serde_support::ipc_timeout_response( request_id, - format!( - "IPC handler timed out after {}ms: {}", - dispatch_timeout.as_millis(), - request.method - ), - IPC_HANDLER_TIMEOUT_CODE.to_string(), - serde_json::json!({ - "timeoutMs": dispatch_timeout.as_millis(), - "method": request.method - }), + request.method, + dispatch_timeout, + queue_delay, ), } } @@ -240,7 +242,7 @@ async fn dispatch_ipc_handler( PromiseState::Pending if crate::modules::dialog_async::has_pending_dialogs() => { // A dialog is pending on another thread — yield briefly and // retry rather than burning through the iteration budget. - std::thread::sleep(std::time::Duration::from_millis(10)); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; continue; } PromiseState::Pending if iterations < MAX_JOB_ITERATIONS => continue, diff --git a/crates/volt-runner/src/js_runtime/requests.rs b/crates/volt-runner/src/js_runtime/requests.rs index 77f618d..0423bd0 100644 --- a/crates/volt-runner/src/js_runtime/requests.rs +++ b/crates/volt-runner/src/js_runtime/requests.rs @@ -1,5 +1,5 @@ use std::sync::mpsc::Sender; -use std::time::Duration; +use std::time::{Duration, Instant}; use serde_json::Value as JsonValue; use volt_core::ipc::IpcResponse; @@ -41,6 +41,7 @@ pub(super) enum RuntimeRequest { DispatchIpc { raw: String, timeout: Duration, + deadline: Instant, response_tx: Sender, }, DispatchNativeEvent { diff --git a/crates/volt-runner/src/js_runtime/serde_support.rs b/crates/volt-runner/src/js_runtime/serde_support.rs index 4f02238..ab253b3 100644 --- a/crates/volt-runner/src/js_runtime/serde_support.rs +++ b/crates/volt-runner/src/js_runtime/serde_support.rs @@ -1,6 +1,8 @@ +use std::time::Duration; + use boa_engine::{Context, JsError, JsValue}; use serde_json::Value as JsonValue; -use volt_core::ipc::{IPC_HANDLER_ERROR_CODE, IpcResponse}; +use volt_core::ipc::{IPC_HANDLER_ERROR_CODE, IPC_HANDLER_TIMEOUT_CODE, IpcResponse}; pub(super) fn js_error(error: JsError) -> String { error.to_string() @@ -59,6 +61,27 @@ pub(super) fn response_for_dispatch_payload(request_id: String, payload: JsonVal IpcResponse::error_with_code(request_id, error_message, error_code) } +pub(super) fn ipc_timeout_response( + request_id: String, + method: String, + timeout: Duration, + queue_delay: Duration, +) -> IpcResponse { + IpcResponse::error_with_details( + request_id, + format!( + "IPC handler timed out after {}ms: {method}", + timeout.as_millis() + ), + IPC_HANDLER_TIMEOUT_CODE.to_string(), + serde_json::json!({ + "timeoutMs": timeout.as_millis(), + "method": method, + "queueDelayMs": queue_delay.as_millis() + }), + ) +} + pub(crate) fn extract_ipc_method(raw: &str) -> String { match serde_json::from_str::(raw) { Ok(value) => value @@ -101,4 +124,27 @@ mod tests { ); assert_eq!(response.error_code.as_deref(), Some(IPC_HANDLER_ERROR_CODE)); } + + #[test] + fn ipc_timeout_response_preserves_explicit_queue_delay() { + let response = ipc_timeout_response( + "req-1".to_string(), + "slow".to_string(), + Duration::from_millis(20), + Duration::from_millis(7), + ); + + assert_eq!( + response.error_code.as_deref(), + Some(IPC_HANDLER_TIMEOUT_CODE) + ); + assert_eq!( + response + .error_details + .as_ref() + .and_then(|details| details.get("queueDelayMs")) + .and_then(serde_json::Value::as_u64), + Some(7) + ); + } } diff --git a/crates/volt-runner/src/js_runtime/tests/native_ipc/roundtrip.rs b/crates/volt-runner/src/js_runtime/tests/native_ipc/roundtrip.rs index 53b91ee..bb6e294 100644 --- a/crates/volt-runner/src/js_runtime/tests/native_ipc/roundtrip.rs +++ b/crates/volt-runner/src/js_runtime/tests/native_ipc/roundtrip.rs @@ -1,6 +1,8 @@ -use std::time::Duration; +use std::sync::mpsc; +use std::time::{Duration, Instant}; use super::*; +use crate::js_runtime::requests::RuntimeRequest; #[test] fn ipc_roundtrip_supports_sync_and_async_handlers() { @@ -170,6 +172,68 @@ fn ipc_dispatch_timeout_is_end_to_end_for_sync_handlers() { ); } +#[test] +fn queued_ipc_requests_consume_the_same_end_to_end_timeout_budget() { + let runtime = JsRuntimeManager::start().expect("js runtime start"); + let client = runtime.client(); + + client + .eval_promise_string( + "(async () => { + const { ipcMain } = await import('volt:ipc'); + ipcMain.handle('busy-sync', () => { + const startedAt = Date.now(); + while ((Date.now() - startedAt) < 50) { + } + return 'done'; + }); + ipcMain.handle('fast', () => 'ok'); + return 'registered'; + })()", + ) + .expect("register busy and fast handlers"); + + let busy_timeout = Duration::from_millis(200); + let (busy_response_tx, busy_response_rx) = mpsc::channel(); + client + .request_tx + .send(RuntimeRequest::DispatchIpc { + raw: r#"{"id":"busy-1","method":"busy-sync","args":null}"#.to_string(), + timeout: busy_timeout, + deadline: Instant::now() + busy_timeout, + response_tx: busy_response_tx, + }) + .expect("queue busy request"); + + let response = client + .dispatch_ipc_message( + r#"{"id":"queued-timeout-1","method":"fast","args":null}"#, + Duration::from_millis(20), + ) + .expect("queued request response"); + + assert_eq!(response.id, "queued-timeout-1"); + assert!(response.result.is_none()); + assert_eq!( + response.error_code.as_deref(), + Some(IPC_HANDLER_TIMEOUT_CODE) + ); + assert!( + response + .error_details + .as_ref() + .and_then(|details| details.get("queueDelayMs")) + .and_then(serde_json::Value::as_u64) + .unwrap_or_default() + > 0 + ); + + let busy_response = busy_response_rx + .recv_timeout(Duration::from_secs(1)) + .expect("busy response"); + assert_eq!(busy_response.result, Some(serde_json::json!("done"))); +} + #[test] fn ipc_roundtrip_rejects_payload_over_limit() { let runtime = JsRuntimeManager::start().expect("js runtime start"); diff --git a/crates/volt-runner/src/js_runtime/worker.rs b/crates/volt-runner/src/js_runtime/worker.rs index 123a250..121db2c 100644 --- a/crates/volt-runner/src/js_runtime/worker.rs +++ b/crates/volt-runner/src/js_runtime/worker.rs @@ -202,6 +202,7 @@ fn execute_request( RuntimeRequest::DispatchIpc { raw, timeout, + deadline, response_tx, } => { let response = tokio_runtime.block_on(async { @@ -211,6 +212,7 @@ fn execute_request( ipc_state, &raw, timeout, + deadline, ) .await; let _ = super::bootstrap::run_jobs(context, job_executor).await; diff --git a/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs b/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs index 89f1b0b..790aed1 100644 --- a/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs +++ b/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::panic::{self, AssertUnwindSafe}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; @@ -84,7 +85,14 @@ impl LifecycleBus { .unwrap_or_default(); for handler in handlers { - handler(&event); + if let Err(payload) = panic::catch_unwind(AssertUnwindSafe(|| handler(&event))) { + tracing::error!( + panic = %panic_payload_message(&payload), + plugin_id = %event.plugin_id, + new_state = ?event.new_state, + "plugin lifecycle subscriber panicked" + ); + } } } @@ -101,6 +109,16 @@ impl LifecycleBus { } } +fn panic_payload_message(payload: &Box) -> String { + if let Some(message) = payload.downcast_ref::<&str>() { + return (*message).to_string(); + } + if let Some(message) = payload.downcast_ref::() { + return message.clone(); + } + "unknown panic payload".to_string() +} + fn topic_matches(topic: LifecycleTopic, event: &PluginLifecycleEvent) -> bool { match topic { LifecycleTopic::All => true, diff --git a/crates/volt-runner/src/plugin_manager/process/child/messaging.rs b/crates/volt-runner/src/plugin_manager/process/child/messaging.rs index 276a413..bf04c28 100644 --- a/crates/volt-runner/src/plugin_manager/process/child/messaging.rs +++ b/crates/volt-runner/src/plugin_manager/process/child/messaging.rs @@ -41,14 +41,19 @@ pub(super) fn send_and_wait( .waiters .lock() .map(|mut waiters| waiters.remove(&message.id)); - let code = if matches!(error, mpsc::RecvTimeoutError::Timeout) { - "TIMEOUT" - } else { - PLUGIN_RUNTIME_ERROR_CODE + let (code, message_text) = match error { + mpsc::RecvTimeoutError::Timeout => ( + "TIMEOUT", + format!("plugin did not respond in {}ms", timeout.as_millis()), + ), + mpsc::RecvTimeoutError::Disconnected => ( + PLUGIN_RUNTIME_ERROR_CODE, + "plugin transport closed before a response was received".to_string(), + ), }; PluginRuntimeError { code: code.to_string(), - message: format!("plugin did not respond in {}ms", timeout.as_millis()), + message: message_text, } }) } diff --git a/crates/volt-runner/src/plugin_manager/process/io.rs b/crates/volt-runner/src/plugin_manager/process/io.rs index 99408cd..d9c4ce2 100644 --- a/crates/volt-runner/src/plugin_manager/process/io.rs +++ b/crates/volt-runner/src/plugin_manager/process/io.rs @@ -146,6 +146,7 @@ pub(super) fn spawn_exit_watcher(process: Arc) { .ok() .and_then(|mut child| child.wait().ok()) .and_then(|status| status.code()); + drain_waiters(&process.waiters); notify_exit(&process.exit, ProcessExitInfo { code: exit_code }); }); } @@ -199,8 +200,10 @@ fn read_plugin_stdout(process: Arc, stdout: ChildStdout loop { let message = match read_wire_message(&mut reader) { Ok(Some(message)) => message, - Ok(None) => return, - Err(_) => return, + Ok(None) | Err(_) => { + drain_waiters(&process.waiters); + return; + } }; if message.message_type == WireMessageType::Signal && message.method == "ready" { @@ -246,6 +249,12 @@ fn notify_exit(exit_state: &ExitState, exit: ProcessExitInfo) { } } +pub(super) fn drain_waiters(waiters: &Mutex>>) { + if let Ok(mut waiters) = waiters.lock() { + waiters.clear(); + } +} + fn read_wire_message(reader: &mut BufReader) -> std::io::Result> { let mut len_buf = [0_u8; 4]; match reader.read_exact(&mut len_buf) { @@ -271,3 +280,23 @@ fn read_wire_message(reader: &mut BufReader) -> std::io::Result { + const cleanups: Array<() => void> = []; + + beforeEach(() => { + resetIpcRuntimeState(); + resetRuntimeModuleState(); + }); + + afterEach(() => { + for (const cleanup of cleanups.splice(0)) { + cleanup(); + } + resetIpcRuntimeState(); + resetRuntimeModuleState(); + }); + + it('loads the volt:bench dev shim for backend handlers', async () => { + const project = createTempProject({ + 'backend.ts': ` + import { ipcMain } from 'volt:ipc'; + import * as bench from 'volt:bench'; + + ipcMain.handle('dev-backend:bench', async () => { + const profile = await bench.analyticsProfile({ datasetSize: 1_200 }); + const workflow = await bench.runWorkflowBenchmark({ batchSize: 800, passes: 2 }); + return { + datasetSize: profile.datasetSize, + batchSize: workflow.batchSize, + pipelineLength: workflow.pipeline.length, + }; + }); + `, + 'tsconfig.json': JSON.stringify({ + compilerOptions: { + target: 'ES2022', + module: 'ESNext', + moduleResolution: 'bundler', + strict: true, + }, + }), + }); + cleanups.push(project.cleanup); + + configureRuntimeModuleState({ + projectRoot: project.rootDir, + defaultWindowId: 'window-dev', + nativeRuntime: { windowEvalScript: vi.fn() }, + }); + + const backendLoadState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(backendLoadState.dispose); + + const response = await ipcMain.processRequest( + 'req-bench', + 'dev-backend:bench', + null, + { timeoutMs: 200 }, + ); + expect(response).toEqual({ + id: 'req-bench', + result: { + datasetSize: 1_200, + batchSize: 800, + pipelineLength: 5, + }, + }); + }); + + it('loads the volt:plugins dev shim for backend handlers', async () => { + const project = createTempProject({ + 'backend.ts': ` + import { ipcMain } from 'volt:ipc'; + import * as plugins from 'volt:plugins'; + + ipcMain.handle('dev-backend:plugins', async () => { + const states = await plugins.getStates(); + const issues = await plugins.getDiscoveryIssues(); + return { + stateCount: states.length, + issueCount: issues.length, + }; + }); + `, + 'tsconfig.json': JSON.stringify({ + compilerOptions: { + target: 'ES2022', + module: 'ESNext', + moduleResolution: 'bundler', + strict: true, + }, + }), + }); + cleanups.push(project.cleanup); + + configureRuntimeModuleState({ + projectRoot: project.rootDir, + defaultWindowId: 'window-dev', + nativeRuntime: { windowEvalScript: vi.fn() }, + }); + + const backendLoadState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(backendLoadState.dispose); + + const response = await ipcMain.processRequest( + 'req-plugins', + 'dev-backend:plugins', + null, + { timeoutMs: 200 }, + ); + expect(response).toEqual({ + id: 'req-plugins', + result: { + stateCount: 0, + issueCount: 0, + }, + }); + }); + + it('enforces permissions in clipboard, shell, and secureStorage dev shims', async () => { + const project = createTempProject({ + 'backend.ts': ` + import { ipcMain } from 'volt:ipc'; + import * as clipboard from 'volt:clipboard'; + import * as shell from 'volt:shell'; + import * as secureStorage from 'volt:secureStorage'; + + ipcMain.handle('dev-backend:permissions', async () => { + const failures: string[] = []; + + try { + clipboard.readText(); + } catch (error) { + failures.push(String(error)); + } + + try { + shell.showItemInFolder('C:/tmp/example.txt'); + } catch (error) { + failures.push(String(error)); + } + + try { + await secureStorage.has('token'); + } catch (error) { + failures.push(String(error)); + } + + return failures; + }); + `, + 'tsconfig.json': JSON.stringify({ + compilerOptions: { + target: 'ES2022', + module: 'ESNext', + moduleResolution: 'bundler', + strict: true, + }, + }), + }); + cleanups.push(project.cleanup); + + configureRuntimeModuleState({ + projectRoot: project.rootDir, + defaultWindowId: 'window-dev', + nativeRuntime: { windowEvalScript: vi.fn() }, + permissions: [], + }); + + const backendLoadState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(backendLoadState.dispose); + + const response = await ipcMain.processRequest( + 'req-permissions', + 'dev-backend:permissions', + null, + { timeoutMs: 200 }, + ); + expect(response).toEqual({ + id: 'req-permissions', + result: [ + "Error: [volt:clipboard] Permission denied: clipboard.readText() requires 'clipboard' in volt.config.ts permissions.", + "Error: [volt:shell] Permission denied: shell.showItemInFolder() requires 'shell' in volt.config.ts permissions.", + "Error: [volt:secureStorage] Permission denied: secureStorage.has() requires 'secureStorage' in volt.config.ts permissions.", + ], + }); + }); + + it('enforces permissions and SSRF protections in the dev http shim', async () => { + const project = createTempProject({ + 'backend.ts': ` + import { ipcMain } from 'volt:ipc'; + import { fetch } from 'volt:http'; + + ipcMain.handle('dev-backend:http-guard', async () => { + const denied = await fetch({ url: 'https://example.com' }) + .then(() => 'unexpected') + .catch((error) => String(error)); + + return { denied }; + }); + `, + 'tsconfig.json': JSON.stringify({ + compilerOptions: { + target: 'ES2022', + module: 'ESNext', + moduleResolution: 'bundler', + strict: true, + }, + }), + }); + cleanups.push(project.cleanup); + + configureRuntimeModuleState({ + projectRoot: project.rootDir, + defaultWindowId: 'window-dev', + nativeRuntime: { windowEvalScript: vi.fn() }, + permissions: [], + }); + + const backendLoadState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(backendLoadState.dispose); + + const denied = await ipcMain.processRequest( + 'req-http-denied', + 'dev-backend:http-guard', + null, + { timeoutMs: 200 }, + ); + expect(denied).toEqual({ + id: 'req-http-denied', + result: { + denied: "Error: [volt:http] Permission denied: http.fetch() requires 'http' in volt.config.ts permissions.", + }, + }); + + writeFileSync( + join(project.rootDir, 'backend.ts'), + ` + import { ipcMain } from 'volt:ipc'; + import { fetch } from 'volt:http'; + + ipcMain.handle('dev-backend:http-guard', async () => { + const blocked = await fetch({ url: 'http://127.0.0.1:8080' }) + .then(() => 'unexpected') + .catch((error) => String(error)); + + return { blocked }; + }); + `, + 'utf8', + ); + + configureRuntimeModuleState({ + projectRoot: project.rootDir, + defaultWindowId: 'window-dev', + nativeRuntime: { windowEvalScript: vi.fn() }, + permissions: ['http'], + }); + resetIpcRuntimeState(); + const reloadedState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(reloadedState.dispose); + + const blocked = await ipcMain.processRequest( + 'req-http-blocked', + 'dev-backend:http-guard', + null, + { timeoutMs: 200 }, + ); + expect(blocked).toEqual({ + id: 'req-http-blocked', + result: { + blocked: "Error: [volt:http] HTTP request blocked in dev mode: host '127.0.0.1' is not allowed.", + }, + }); + }); +}); diff --git a/packages/volt-cli/src/__tests__/dev-backend-test-utils.ts b/packages/volt-cli/src/__tests__/dev-backend-test-utils.ts new file mode 100644 index 0000000..b595614 --- /dev/null +++ b/packages/volt-cli/src/__tests__/dev-backend-test-utils.ts @@ -0,0 +1,22 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; + +export interface TempProject { + rootDir: string; + cleanup: () => void; +} + +export function createTempProject(files: Record): TempProject { + const rootDir = mkdtempSync(join(process.cwd(), '.tmp-dev-backend-test-')); + for (const [relativePath, contents] of Object.entries(files)) { + const absolutePath = join(rootDir, relativePath); + mkdirSync(dirname(absolutePath), { recursive: true }); + writeFileSync(absolutePath, contents, 'utf8'); + } + return { + rootDir, + cleanup: () => { + rmSync(rootDir, { recursive: true, force: true }); + }, + }; +} diff --git a/packages/volt-cli/src/__tests__/dev-backend.test.ts b/packages/volt-cli/src/__tests__/dev-backend.test.ts index 77e872b..815283e 100644 --- a/packages/volt-cli/src/__tests__/dev-backend.test.ts +++ b/packages/volt-cli/src/__tests__/dev-backend.test.ts @@ -1,5 +1,5 @@ -import { existsSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; +import { existsSync, utimesSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; import { ipcMain } from 'voltkit'; import { __testOnly, loadBackendEntrypointForDev } from '../commands/dev/backend.js'; @@ -8,26 +8,7 @@ import { configureRuntimeModuleState, resetRuntimeModuleState, } from '../commands/dev/runtime-modules/shared.js'; - -interface TempProject { - rootDir: string; - cleanup: () => void; -} - -function createTempProject(files: Record): TempProject { - const rootDir = mkdtempSync(join(process.cwd(), '.tmp-dev-backend-test-')); - for (const [relativePath, contents] of Object.entries(files)) { - const absolutePath = join(rootDir, relativePath); - mkdirSync(dirname(absolutePath), { recursive: true }); - writeFileSync(absolutePath, contents, 'utf8'); - } - return { - rootDir, - cleanup: () => { - rmSync(rootDir, { recursive: true, force: true }); - }, - }; -} +import { createTempProject } from './dev-backend-test-utils.js'; describe('dev backend bootstrap', () => { const cleanups: Array<() => void> = []; @@ -134,21 +115,10 @@ describe('dev backend bootstrap', () => { expect(script).toContain('demo:event'); }); - it('loads the volt:bench dev shim for backend handlers', async () => { + it('fails fast when backend imports unsupported volt:* modules', async () => { const project = createTempProject({ 'backend.ts': ` - import { ipcMain } from 'volt:ipc'; - import * as bench from 'volt:bench'; - - ipcMain.handle('dev-backend:bench', async () => { - const profile = await bench.analyticsProfile({ datasetSize: 1_200 }); - const workflow = await bench.runWorkflowBenchmark({ batchSize: 800, passes: 2 }); - return { - datasetSize: profile.datasetSize, - batchSize: workflow.batchSize, - pipelineLength: workflow.pipeline.length, - }; - }); + import 'volt:unknown-module'; `, 'tsconfig.json': JSON.stringify({ compilerOptions: { @@ -167,29 +137,16 @@ describe('dev backend bootstrap', () => { nativeRuntime: { windowEvalScript: vi.fn() }, }); - const backendLoadState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); - cleanups.push(backendLoadState.dispose); - - const response = await ipcMain.processRequest( - 'req-bench', - 'dev-backend:bench', - null, - { timeoutMs: 200 }, - ); - expect(response).toEqual({ - id: 'req-bench', - result: { - datasetSize: 1_200, - batchSize: 800, - pipelineLength: 5, - }, - }); + await expect( + loadBackendEntrypointForDev(project.rootDir, './backend.ts'), + ).rejects.toThrow('Unsupported backend module in dev mode: volt:unknown-module'); }); - it('fails fast when backend imports unsupported volt:* modules', async () => { + it('clears existing IPC handlers before backend reload', async () => { const project = createTempProject({ 'backend.ts': ` - import 'volt:unknown-module'; + import { ipcMain } from 'volt:ipc'; + ipcMain.handle('dev-backend:reloadable', () => ({ version: 1 })); `, 'tsconfig.json': JSON.stringify({ compilerOptions: { @@ -208,9 +165,38 @@ describe('dev backend bootstrap', () => { nativeRuntime: { windowEvalScript: vi.fn() }, }); - await expect( - loadBackendEntrypointForDev(project.rootDir, './backend.ts'), - ).rejects.toThrow('Unsupported backend module in dev mode: volt:unknown-module'); + const backendLoadState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(backendLoadState.dispose); + + const initial = await ipcMain.processRequest( + 'req-reload-1', + 'dev-backend:reloadable', + null, + { timeoutMs: 200 }, + ); + expect(initial).toEqual({ + id: 'req-reload-1', + result: { version: 1 }, + }); + + writeFileSync(join(project.rootDir, 'backend.ts'), ` + import { ipcMain } from 'volt:ipc'; + ipcMain.handle('dev-backend:reloadable', () => ({ version: 2 })); + `, 'utf8'); + + const reloadedState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(reloadedState.dispose); + + const reloaded = await ipcMain.processRequest( + 'req-reload-2', + 'dev-backend:reloadable', + null, + { timeoutMs: 200 }, + ); + expect(reloaded).toEqual({ + id: 'req-reload-2', + result: { version: 2 }, + }); }); it('prunes stale dev backend bundles while preserving fresh ones', () => { diff --git a/packages/volt-cli/src/commands/dev.ts b/packages/volt-cli/src/commands/dev.ts index c35ce37..4239e70 100644 --- a/packages/volt-cli/src/commands/dev.ts +++ b/packages/volt-cli/src/commands/dev.ts @@ -179,6 +179,7 @@ export async function devCommand(options: DevOptions): Promise { projectRoot: cwd, defaultWindowId: windowConfig.jsId, nativeRuntime, + permissions: config.permissions ?? [], }); let backendLoadState: Awaited>; diff --git a/packages/volt-cli/src/commands/dev/backend.ts b/packages/volt-cli/src/commands/dev/backend.ts index e1ae7a1..508df26 100644 --- a/packages/volt-cli/src/commands/dev/backend.ts +++ b/packages/volt-cli/src/commands/dev/backend.ts @@ -3,6 +3,7 @@ import { existsSync, rmSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { resolveBackendEntry } from '../build/backend.js'; +import { resetIpcRuntimeState } from './ipc.js'; import { createScopedTempDirectory, recoverStaleScopedDirectories } from '../../utils/temp-artifacts.js'; const RUNTIME_MODULES_DIR = resolve( @@ -29,6 +30,7 @@ const VOLT_RUNTIME_MODULE_TO_STEM: Record = { 'volt:shell': 'volt-shell', 'volt:notification': 'volt-notification', 'volt:http': 'volt-http', + 'volt:plugins': 'volt-plugins', 'volt:bench': 'volt-bench', 'volt:updater': 'volt-updater', }; @@ -129,6 +131,7 @@ export async function loadBackendEntrypointForDev( }); const backendUrl = `${pathToFileURL(backendBundlePath).href}?t=${Date.now()}`; + resetIpcRuntimeState(); await import(backendUrl); return { @@ -147,6 +150,7 @@ export async function loadBackendEntrypointForDev( return; } try { + resetIpcRuntimeState(); const url = `${pathToFileURL(backendBundlePath).href}?t=${Date.now()}`; await import(url); console.log('[volt] Backend reloaded.'); diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts b/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts index 0080b1f..ebcaf1b 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts @@ -8,6 +8,7 @@ interface DevRuntimeModuleState { projectRoot: string; defaultWindowId: string; nativeRuntime: NativeScriptBridge | null; + permissions: string[]; } const GLOBAL_RUNTIME_STATE_KEY = '__VOLT_DEV_RUNTIME_MODULE_STATE__'; @@ -17,6 +18,7 @@ function createDefaultRuntimeState(): DevRuntimeModuleState { projectRoot: process.cwd(), defaultWindowId: '', nativeRuntime: null, + permissions: [], }; } @@ -45,18 +47,37 @@ export function configureRuntimeModuleState(next: Partial if (next.nativeRuntime !== undefined) { runtimeState.nativeRuntime = next.nativeRuntime; } + if (Array.isArray(next.permissions)) { + runtimeState.permissions = next.permissions + .filter((permission): permission is string => typeof permission === 'string') + .map((permission) => permission.trim()) + .filter((permission) => permission.length > 0); + } } export function resetRuntimeModuleState(): void { runtimeState.projectRoot = process.cwd(); runtimeState.defaultWindowId = ''; runtimeState.nativeRuntime = null; + runtimeState.permissions = []; } export function projectRoot(): string { return runtimeState.projectRoot; } +export function runtimePermissions(): ReadonlySet { + return new Set(runtimeState.permissions); +} + +export function ensureDevPermission(permission: string, apiName: string): void { + if (!runtimePermissions().has(permission)) { + throw new Error( + `Permission denied: ${apiName} requires '${permission}' in volt.config.ts permissions.`, + ); + } +} + export function resolveProjectScopedPath(path: string, namespace: string): string { const trimmedPath = path.trim(); if (trimmedPath.length === 0) { diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-clipboard.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-clipboard.ts index 0c60737..962c072 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-clipboard.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-clipboard.ts @@ -1,10 +1,12 @@ import { clipboard } from 'voltkit'; +import { ensureDevPermission } from './shared.js'; export function readText(): string { + ensureDevPermission('clipboard', 'clipboard.readText()'); return clipboard.readText(); } export function writeText(text: string): void { + ensureDevPermission('clipboard', 'clipboard.writeText()'); clipboard.writeText(text); } - diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts index 3256bae..b526ad9 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts @@ -1,3 +1,7 @@ +import { lookup } from 'node:dns/promises'; +import { isIP } from 'node:net'; +import { ensureDevPermission } from './shared.js'; + interface HttpFetchRequest { url: string; method?: string; @@ -13,6 +17,15 @@ interface HttpFetchResponse { json(): Promise; } +const BLOCKED_HOSTNAMES = new Set([ + 'localhost', + '127.0.0.1', + '0.0.0.0', + '::1', + '169.254.169.254', + 'metadata.google.internal', +]); + function normalizeBody(body: unknown): unknown { if (body === null || body === undefined) { return undefined; @@ -43,7 +56,85 @@ function normalizeHeaders(headers: Headers): Record { return normalized; } +function createBlockedHttpError(reason: string): Error { + return new Error(`HTTP request blocked in dev mode: ${reason}.`); +} + +function normalizeRequestUrl(url: string): URL { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + throw new Error(`Invalid HTTP URL: ${url}`); + } + + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + throw createBlockedHttpError(`unsupported protocol '${parsed.protocol}'`); + } + if (parsed.username || parsed.password) { + throw createBlockedHttpError('embedded credentials are not allowed'); + } + return parsed; +} + +function isBlockedHostname(hostname: string): boolean { + const normalized = hostname.trim().toLowerCase(); + return BLOCKED_HOSTNAMES.has(normalized) || normalized.endsWith('.localhost'); +} + +function isPrivateIpAddress(address: string): boolean { + const family = isIP(address); + if (family === 4) { + const octets = address.split('.').map((part) => Number.parseInt(part, 10)); + if (octets.length !== 4 || octets.some((part) => Number.isNaN(part))) { + return true; + } + const [a, b] = octets; + return a === 0 + || a === 10 + || a === 127 + || (a === 169 && b === 254) + || (a === 172 && b >= 16 && b <= 31) + || (a === 192 && b === 168); + } + + if (family === 6) { + const normalized = address.toLowerCase(); + return normalized === '::1' + || normalized.startsWith('fe8') + || normalized.startsWith('fe9') + || normalized.startsWith('fea') + || normalized.startsWith('feb') + || normalized.startsWith('fc') + || normalized.startsWith('fd') + || normalized.startsWith('::ffff:127.'); + } + + return true; +} + +async function ensureSafeRequestTarget(url: URL): Promise { + if (isBlockedHostname(url.hostname)) { + throw createBlockedHttpError(`host '${url.hostname}' is not allowed`); + } + + const addresses = isIP(url.hostname) + ? [url.hostname] + : (await lookup(url.hostname, { all: true, verbatim: true })).map((entry) => entry.address); + + if (addresses.length === 0) { + throw createBlockedHttpError(`host '${url.hostname}' did not resolve to an address`); + } + if (addresses.some((address) => isPrivateIpAddress(address))) { + throw createBlockedHttpError(`host '${url.hostname}' resolved to a private or local address`); + } +} + export async function fetch(request: HttpFetchRequest): Promise { + ensureDevPermission('http', 'http.fetch()'); + const targetUrl = normalizeRequestUrl(request.url); + await ensureSafeRequestTarget(targetUrl); + const controller = new AbortController(); const timeoutMs = request.timeoutMs; const timeout = typeof timeoutMs === 'number' && Number.isFinite(timeoutMs) && timeoutMs > 0 @@ -51,7 +142,7 @@ export async function fetch(request: HttpFetchRequest): Promise void; + +const listeners = new Map>(); + +function normalizePluginId(pluginId: string): string { + const normalized = pluginId.trim(); + if (!normalized) { + throw new Error('plugin id must be a non-empty string'); + } + return normalized; +} + +function normalizeGrantId(grantId: string): string { + const normalized = grantId.trim(); + if (!normalized) { + throw new Error('grant id must be a non-empty string'); + } + return normalized; +} + +function normalizeSurface(surface: string): string { + const normalized = surface.trim(); + if (!normalized) { + throw new Error('surface must be a non-empty string'); + } + return normalized; +} + +function normalizeEventName(eventName: string): PluginLifecycleEventName { + if ( + eventName === 'plugin:lifecycle' + || eventName === 'plugin:failed' + || eventName === 'plugin:activated' + ) { + return eventName; + } + throw new Error(`unsupported plugin event '${eventName}'`); +} + +export async function delegateGrant(pluginId: string, grantId: string): Promise { + normalizePluginId(pluginId); + normalizeGrantId(grantId); +} + +export async function revokeGrant(pluginId: string, grantId: string): Promise { + normalizePluginId(pluginId); + normalizeGrantId(grantId); +} + +export async function prefetchFor(surface: string): Promise { + normalizeSurface(surface); +} + +export async function getStates(): Promise { + return []; +} + +export async function getPluginState(pluginId: string): Promise { + normalizePluginId(pluginId); + return null; +} + +export async function getErrors(): Promise { + return []; +} + +export async function getPluginErrors(pluginId: string): Promise { + normalizePluginId(pluginId); + return []; +} + +export async function getDiscoveryIssues(): Promise { + return []; +} + +export async function retryPlugin(pluginId: string): Promise { + normalizePluginId(pluginId); +} + +export async function enablePlugin(pluginId: string): Promise { + normalizePluginId(pluginId); +} + +export function on(eventName: string, handler: PluginEventHandler): void { + const normalized = normalizeEventName(eventName); + const existing = listeners.get(normalized); + if (existing) { + existing.add(handler); + return; + } + listeners.set(normalized, new Set([handler])); +} + +export function off(eventName: string, handler: PluginEventHandler): void { + const normalized = normalizeEventName(eventName); + listeners.get(normalized)?.delete(handler); +} diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts index c69070c..b47fcba 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts @@ -1,5 +1,5 @@ import { existsSync, readFileSync, writeFileSync } from 'node:fs'; -import { resolveProjectScopedPath } from './shared.js'; +import { ensureDevPermission, resolveProjectScopedPath } from './shared.js'; const storage = new Map(); let loaded = false; @@ -49,18 +49,21 @@ function normalizeKey(key: string): string { } export async function set(key: string, value: string): Promise { + ensureDevPermission('secureStorage', 'secureStorage.set()'); loadStorage(); storage.set(normalizeKey(key), value); persistStorage(); } export async function get(key: string): Promise { + ensureDevPermission('secureStorage', 'secureStorage.get()'); loadStorage(); const normalizedKey = normalizeKey(key); return storage.get(normalizedKey) ?? null; } async function remove(key: string): Promise { + ensureDevPermission('secureStorage', 'secureStorage.delete()'); loadStorage(); storage.delete(normalizeKey(key)); persistStorage(); @@ -69,7 +72,7 @@ async function remove(key: string): Promise { export { remove as delete }; export async function has(key: string): Promise { + ensureDevPermission('secureStorage', 'secureStorage.has()'); loadStorage(); return storage.has(normalizeKey(key)); } - diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-shell.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-shell.ts index 3ed8ed0..db64168 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-shell.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-shell.ts @@ -1,10 +1,12 @@ import { shell } from 'voltkit'; +import { ensureDevPermission } from './shared.js'; export async function openExternal(url: string): Promise { + ensureDevPermission('shell', 'shell.openExternal()'); await shell.openExternal(url); } export function showItemInFolder(path: string): void { + ensureDevPermission('shell', 'shell.showItemInFolder()'); shell.showItemInFolder(path); } - From 5fa8ca001e86c1fcc156a2853a60035b7378f79f Mon Sep 17 00:00:00 2001 From: MerhiOPS Date: Mon, 16 Mar 2026 12:48:21 +0200 Subject: [PATCH 2/4] fix low severity dev shim error parity --- .../src/plugin_manager/lifecycle.rs | 5 ++- .../commands/dev/runtime-modules/shared.ts | 19 +++++++----- .../commands/dev/runtime-modules/volt-db.ts | 8 ++--- .../commands/dev/runtime-modules/volt-fs.ts | 31 +++++++++++-------- .../runtime-modules/volt-global-shortcut.ts | 11 ++++--- .../commands/dev/runtime-modules/volt-http.ts | 6 ++-- .../commands/dev/runtime-modules/volt-menu.ts | 6 ++-- .../dev/runtime-modules/volt-plugins.ts | 10 +++--- .../runtime-modules/volt-secure-storage.ts | 4 +-- .../commands/dev/runtime-modules/volt-tray.ts | 6 ++-- 10 files changed, 62 insertions(+), 44 deletions(-) diff --git a/crates/volt-runner/src/plugin_manager/lifecycle.rs b/crates/volt-runner/src/plugin_manager/lifecycle.rs index 0c514e1..3377e01 100644 --- a/crates/volt-runner/src/plugin_manager/lifecycle.rs +++ b/crates/volt-runner/src/plugin_manager/lifecycle.rs @@ -164,9 +164,12 @@ fn trim_error_history(errors: &mut Vec, max_errors: usize) { } fn is_valid_transition(current: PluginState, next: PluginState) -> bool { - if current == next || next == PluginState::Failed || next == PluginState::Disabled { + if next == PluginState::Failed || next == PluginState::Disabled { return true; } + if current == next { + return current == PluginState::Terminated; + } matches!( (current, next), diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts b/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts index ebcaf1b..c79b369 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/shared.ts @@ -70,9 +70,14 @@ export function runtimePermissions(): ReadonlySet { return new Set(runtimeState.permissions); } +export function devModuleError(moduleName: string, message: string): Error { + return new Error(`[volt:${moduleName}] ${message}`); +} + export function ensureDevPermission(permission: string, apiName: string): void { if (!runtimePermissions().has(permission)) { - throw new Error( + throw devModuleError( + permission, `Permission denied: ${apiName} requires '${permission}' in volt.config.ts permissions.`, ); } @@ -81,10 +86,10 @@ export function ensureDevPermission(permission: string, apiName: string): void { export function resolveProjectScopedPath(path: string, namespace: string): string { const trimmedPath = path.trim(); if (trimmedPath.length === 0) { - throw new Error('Path must be a non-empty string.'); + throw devModuleError('dev', 'Path must be a non-empty string.'); } if (trimmedPath.includes('\0')) { - throw new Error('Path must not include null bytes.'); + throw devModuleError('dev', 'Path must not include null bytes.'); } const safePath = trimmedPath.replace(/\\/g, '/').replace(/^\/+/, ''); @@ -96,7 +101,7 @@ export function resolveProjectScopedPath(path: string, namespace: string): strin relativePath !== '' && (relativePath.startsWith('..') || isAbsolute(relativePath)) ) { - throw new Error(`Path traversal is not allowed: ${path}`); + throw devModuleError('dev', `Path traversal is not allowed: ${path}`); } mkdirSync(dirname(absoluteTargetPath), { recursive: true }); @@ -120,18 +125,18 @@ function resolveEventWindowId(windowId?: string): string { if (runtimeState.defaultWindowId.trim().length > 0) { return runtimeState.defaultWindowId.trim(); } - throw new Error('No target window ID is available for backend event dispatch.'); + throw devModuleError('events', 'No target window ID is available for backend event dispatch.'); } export function emitFrontendEvent(eventName: string, data?: unknown, windowId?: string): void { const event = eventName.trim(); if (event.length === 0) { - throw new Error('Event name must be a non-empty string.'); + throw devModuleError('events', 'Event name must be a non-empty string.'); } const nativeRuntime = runtimeState.nativeRuntime; if (!nativeRuntime) { - throw new Error('Native runtime bridge is unavailable for backend event dispatch.'); + throw devModuleError('events', 'Native runtime bridge is unavailable for backend event dispatch.'); } const targetWindowId = resolveEventWindowId(windowId); diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-db.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-db.ts index c980a5b..557be98 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-db.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-db.ts @@ -1,7 +1,7 @@ import { mkdirSync } from 'node:fs'; import { dirname } from 'node:path'; import { DatabaseSync, type SQLInputValue } from 'node:sqlite'; -import { resolveProjectScopedPath } from './shared.js'; +import { devModuleError, resolveProjectScopedPath } from './shared.js'; interface ExecuteResult { rowsAffected: number; @@ -12,7 +12,7 @@ let openPath: string | null = null; function ensureConnection(): DatabaseSync { if (!connection) { - throw new Error('Database is not open. Call db.open(path) first.'); + throw devModuleError('db', 'Database is not open. Call db.open(path) first.'); } return connection; } @@ -20,7 +20,7 @@ function ensureConnection(): DatabaseSync { function normalizeSql(sql: string): string { const statement = sql.trim(); if (!statement) { - throw new Error('SQL statement must be a non-empty string.'); + throw devModuleError('db', 'SQL statement must be a non-empty string.'); } return statement; } @@ -73,7 +73,7 @@ function toJsonRow(row: Record): Record { function resolveDatabasePath(path: string): string { const trimmedPath = path.trim(); if (!trimmedPath) { - throw new Error('Database path must be a non-empty string.'); + throw devModuleError('db', 'Database path must be a non-empty string.'); } if (trimmedPath === ':memory:') { return trimmedPath; diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-fs.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-fs.ts index 0cc9cf3..4f29ea4 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-fs.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-fs.ts @@ -1,5 +1,10 @@ import { fs as frameworkFs } from 'voltkit'; import type { FileInfo, ScopedFs, WatchEvent, FileWatcher } from 'voltkit'; +import { devModuleError } from './shared.js'; + +function invalidScopeError(): Error { + return devModuleError('fs', 'FS_SCOPE_INVALID: grant ID not found or expired'); +} export async function readFile(path: string): Promise { return frameworkFs.readFile(path); @@ -40,61 +45,61 @@ const devScopedHandles = new Map(); export async function scopedReadFile(grantId: string, path: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); return handle.readFile(path); } export async function scopedReadFileBinary(grantId: string, path: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); return handle.readFileBinary(path); } export async function scopedReadDir(grantId: string, path: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); return handle.readDir(path); } export async function scopedStat(grantId: string, path: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); return handle.stat(path); } export async function scopedExists(grantId: string, path: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); return handle.exists(path); } export async function scopedWriteFile(grantId: string, path: string, data: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); await handle.writeFile(path, data); } export async function scopedMkdir(grantId: string, path: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); await handle.mkdir(path); } export async function scopedRemove(grantId: string, path: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); await handle.remove(path); } export async function scopedRename(grantId: string, from: string, to: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); await handle.rename(from, to); } export async function scopedCopy(grantId: string, from: string, to: string): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); await handle.copy(from, to); } @@ -111,13 +116,13 @@ export async function watchStart( export async function watchPoll(watcherId: string): Promise { const watcher = devWatcherHandles.get(watcherId); - if (!watcher) throw new Error('watcher not found'); + if (!watcher) throw devModuleError('fs', 'watcher not found'); return watcher.poll(); } export async function watchClose(watcherId: string): Promise { const watcher = devWatcherHandles.get(watcherId); - if (!watcher) throw new Error('watcher not found'); + if (!watcher) throw devModuleError('fs', 'watcher not found'); await watcher.close(); devWatcherHandles.delete(watcherId); } @@ -129,7 +134,7 @@ export async function scopedWatchStart( debounceMs: number, ): Promise { const handle = devScopedHandles.get(grantId); - if (!handle) throw new Error('FS_SCOPE_INVALID: grant ID not found or expired'); + if (!handle) throw invalidScopeError(); const watcher = await handle.watch(subpath, { recursive, debounceMs }); const id = `dev_watcher_${Date.now()}_${Math.random().toString(36).slice(2)}`; devWatcherHandles.set(id, watcher); diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-global-shortcut.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-global-shortcut.ts index 298bd2e..f83b325 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-global-shortcut.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-global-shortcut.ts @@ -1,4 +1,5 @@ import { globalShortcut } from 'voltkit'; +import { devModuleError } from './shared.js'; type ShortcutHandler = (payload: unknown) => void; @@ -19,7 +20,7 @@ function emitTriggered(payload: unknown): void { export async function register(accelerator: string): Promise { const normalizedAccelerator = accelerator.trim(); if (!normalizedAccelerator) { - throw new Error('Accelerator must be a non-empty string.'); + throw devModuleError('globalShortcut', 'Accelerator must be a non-empty string.'); } const existing = registrations.get(normalizedAccelerator); @@ -34,7 +35,10 @@ export async function register(accelerator: string): Promise { emitTriggered({ id, accelerator: normalizedAccelerator }); }); if (!registered) { - throw new Error(`Global shortcut already registered: ${normalizedAccelerator}`); + throw devModuleError( + 'globalShortcut', + `Global shortcut already registered: ${normalizedAccelerator}`, + ); } registrations.set(normalizedAccelerator, id); @@ -57,7 +61,7 @@ export async function unregisterAll(): Promise { export function on(eventName: 'triggered', handler: (payload: unknown) => void): void { if (eventName !== 'triggered') { - throw new Error(`Unsupported shortcut event "${eventName}".`); + throw devModuleError('globalShortcut', `Unsupported shortcut event "${eventName}".`); } listeners.add(handler); } @@ -68,4 +72,3 @@ export function off(eventName: 'triggered', handler: (payload: unknown) => void) } listeners.delete(handler); } - diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts index b526ad9..18717a1 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-http.ts @@ -1,6 +1,6 @@ import { lookup } from 'node:dns/promises'; import { isIP } from 'node:net'; -import { ensureDevPermission } from './shared.js'; +import { devModuleError, ensureDevPermission } from './shared.js'; interface HttpFetchRequest { url: string; @@ -57,7 +57,7 @@ function normalizeHeaders(headers: Headers): Record { } function createBlockedHttpError(reason: string): Error { - return new Error(`HTTP request blocked in dev mode: ${reason}.`); + return devModuleError('http', `HTTP request blocked in dev mode: ${reason}.`); } function normalizeRequestUrl(url: string): URL { @@ -65,7 +65,7 @@ function normalizeRequestUrl(url: string): URL { try { parsed = new URL(url); } catch { - throw new Error(`Invalid HTTP URL: ${url}`); + throw devModuleError('http', `Invalid HTTP URL: ${url}`); } if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-menu.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-menu.ts index 4a44a64..cd4c1b0 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-menu.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-menu.ts @@ -1,4 +1,5 @@ import { Menu, type MenuItemOptions } from 'voltkit'; +import { devModuleError } from './shared.js'; type MenuClickHandler = (payload: unknown) => void; @@ -46,7 +47,7 @@ function decorateMenuItem(raw: unknown): MenuItemOptions { function normalizeTemplate(template: unknown): MenuItemOptions[] { if (!Array.isArray(template)) { - throw new Error('Menu template must be an array.'); + throw devModuleError('menu', 'Menu template must be an array.'); } return template.map((item) => decorateMenuItem(item)); } @@ -58,7 +59,7 @@ export async function setAppMenu(template: unknown): Promise { export function on(eventName: 'click', handler: (payload: unknown) => void): void { if (eventName !== 'click') { - throw new Error(`Unsupported menu event "${eventName}".`); + throw devModuleError('menu', `Unsupported menu event "${eventName}".`); } clickHandlers.add(handler); } @@ -69,4 +70,3 @@ export function off(eventName: 'click', handler: (payload: unknown) => void): vo } clickHandlers.delete(handler); } - diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-plugins.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-plugins.ts index 118262c..682dcc3 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-plugins.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-plugins.ts @@ -1,3 +1,5 @@ +import { devModuleError } from './shared.js'; + type PluginLifecycleEventName = 'plugin:lifecycle' | 'plugin:failed' | 'plugin:activated'; type PluginEventHandler = (event: unknown) => void; @@ -6,7 +8,7 @@ const listeners = new Map>(); function normalizePluginId(pluginId: string): string { const normalized = pluginId.trim(); if (!normalized) { - throw new Error('plugin id must be a non-empty string'); + throw devModuleError('plugins', 'plugin id must be a non-empty string'); } return normalized; } @@ -14,7 +16,7 @@ function normalizePluginId(pluginId: string): string { function normalizeGrantId(grantId: string): string { const normalized = grantId.trim(); if (!normalized) { - throw new Error('grant id must be a non-empty string'); + throw devModuleError('plugins', 'grant id must be a non-empty string'); } return normalized; } @@ -22,7 +24,7 @@ function normalizeGrantId(grantId: string): string { function normalizeSurface(surface: string): string { const normalized = surface.trim(); if (!normalized) { - throw new Error('surface must be a non-empty string'); + throw devModuleError('plugins', 'surface must be a non-empty string'); } return normalized; } @@ -35,7 +37,7 @@ function normalizeEventName(eventName: string): PluginLifecycleEventName { ) { return eventName; } - throw new Error(`unsupported plugin event '${eventName}'`); + throw devModuleError('plugins', `unsupported plugin event '${eventName}'`); } export async function delegateGrant(pluginId: string, grantId: string): Promise { diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts index b47fcba..e5ac2e8 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts @@ -1,5 +1,5 @@ import { existsSync, readFileSync, writeFileSync } from 'node:fs'; -import { ensureDevPermission, resolveProjectScopedPath } from './shared.js'; +import { devModuleError, ensureDevPermission, resolveProjectScopedPath } from './shared.js'; const storage = new Map(); let loaded = false; @@ -43,7 +43,7 @@ function persistStorage(): void { function normalizeKey(key: string): string { const normalized = key.trim(); if (!normalized) { - throw new Error('Secure storage key must be a non-empty string.'); + throw devModuleError('secureStorage', 'Secure storage key must be a non-empty string.'); } return normalized; } diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-tray.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-tray.ts index 11fd2f5..b361ed9 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-tray.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-tray.ts @@ -1,4 +1,5 @@ import { Tray } from 'voltkit'; +import { devModuleError } from './shared.js'; type TrayClickHandler = (payload: unknown) => void; @@ -17,7 +18,7 @@ function emitTrayClick(payload: unknown): void { function currentTray(): Tray { if (!tray) { - throw new Error('Tray is not created. Call create() first.'); + throw devModuleError('tray', 'Tray is not created. Call create() first.'); } return tray; } @@ -55,7 +56,7 @@ export function destroy(): void { export function on(eventName: 'click', handler: (payload: unknown) => void): void { if (eventName !== 'click') { - throw new Error(`Unsupported tray event "${eventName}".`); + throw devModuleError('tray', `Unsupported tray event "${eventName}".`); } clickHandlers.add(handler); } @@ -66,4 +67,3 @@ export function off(eventName: 'click', handler: (payload: unknown) => void): vo } clickHandlers.delete(handler); } - From ce3fb69094156ede3420656fb8e5d80d4aa7b5b5 Mon Sep 17 00:00:00 2001 From: MerhiOPS Date: Mon, 16 Mar 2026 12:51:11 +0200 Subject: [PATCH 3/4] fix medium severity plugin lifecycle and storage issues --- crates/volt-core/src/grant_store.rs | 26 +++++- crates/volt-core/src/plugin_grant_registry.rs | 84 +++++++++++------- crates/volt-plugin-host/src/engine.rs | 6 +- crates/volt-plugin-host/src/runtime_state.rs | 40 +++++++-- .../src/runtime_state/tests.rs | 27 +++++- crates/volt-runner/src/ipc_bridge/response.rs | 12 ++- .../src/plugin_manager/discovery.rs | 1 + .../src/plugin_manager/host_api_access.rs | 12 +-- .../src/plugin_manager/host_api_storage.rs | 6 +- .../src/plugin_manager/lifecycle.rs | 1 + .../src/plugin_manager/lifecycle_bus.rs | 88 +++++++++++++++---- crates/volt-runner/src/plugin_manager/mod.rs | 1 + .../runtime/lifecycle/shutdown.rs | 56 ++++++++---- .../plugin_manager/runtime/lifecycle/spawn.rs | 33 ++++++- .../src/plugin_manager/tests/activation.rs | 52 +++++++++++ .../src/plugin_manager/tests/lifecycle.rs | 51 +++++++++++ .../plugin_manager/tests/process_support.rs | 18 ++++ .../src/plugin_manager/watchdog.rs | 5 +- 18 files changed, 426 insertions(+), 93 deletions(-) diff --git a/crates/volt-core/src/grant_store.rs b/crates/volt-core/src/grant_store.rs index 7e0bae1..0a050af 100644 --- a/crates/volt-core/src/grant_store.rs +++ b/crates/volt-core/src/grant_store.rs @@ -81,7 +81,11 @@ pub fn resolve_grant(id: &str) -> Result { /// Revoke a grant, removing it from the store. /// Returns true if the grant existed and was removed. pub fn revoke_grant(id: &str) -> bool { - with_store(|store| store.remove(id).is_some()) + let removed = with_store(|store| store.remove(id).is_some()); + if removed { + crate::plugin_grant_registry::revoke_grant_everywhere(id); + } + removed } /// Clear all grants. Intended for testing and app shutdown. @@ -130,6 +134,26 @@ mod tests { assert!(resolve_grant(&id).is_err()); } + #[test] + fn test_revoke_grant_removes_plugin_delegations() { + let _guard = lock_grant_state(); + let dir = env::temp_dir(); + let id = create_grant(dir).unwrap(); + + crate::plugin_grant_registry::delegate_grant("acme.search", &id).expect("delegate grant"); + assert!(crate::plugin_grant_registry::is_delegated( + "acme.search", + &id + )); + + assert!(revoke_grant(&id)); + + assert!(!crate::plugin_grant_registry::is_delegated( + "acme.search", + &id + )); + } + #[test] fn test_create_grant_rejects_nonexistent_path() { let _guard = lock_grant_state(); diff --git a/crates/volt-core/src/plugin_grant_registry.rs b/crates/volt-core/src/plugin_grant_registry.rs index 15cb870..a18104d 100644 --- a/crates/volt-core/src/plugin_grant_registry.rs +++ b/crates/volt-core/src/plugin_grant_registry.rs @@ -10,7 +10,6 @@ pub struct GrantDelegation { pub grant_id: String, pub plugin_id: String, pub delegated_at: u64, - pub revoked: bool, } #[derive(Debug, Error, PartialEq, Eq)] @@ -49,55 +48,63 @@ pub fn delegate_grant(plugin_id: &str, grant_id: &str) -> Result<(), PluginGrant with_store(|store| { let delegations = store.entry(plugin_id.to_string()).or_default(); - match delegations.get_mut(grant_id) { - Some(delegation) if !delegation.revoked => Err(PluginGrantError::AlreadyDelegated), - Some(delegation) => { - delegation.delegated_at = now_ms(); - delegation.revoked = false; - Ok(()) - } - None => { - delegations.insert( - grant_id.to_string(), - GrantDelegation { - grant_id: grant_id.to_string(), - plugin_id: plugin_id.to_string(), - delegated_at: now_ms(), - revoked: false, - }, - ); - Ok(()) - } + if delegations.contains_key(grant_id) { + Err(PluginGrantError::AlreadyDelegated) + } else { + delegations.insert( + grant_id.to_string(), + GrantDelegation { + grant_id: grant_id.to_string(), + plugin_id: plugin_id.to_string(), + delegated_at: now_ms(), + }, + ); + Ok(()) } }) } pub fn revoke_grant(plugin_id: &str, grant_id: &str) { with_store(|store| { - if let Some(delegations) = store.get_mut(plugin_id) - && let Some(delegation) = delegations.get_mut(grant_id) - { - delegation.revoked = true; + if let Some(delegations) = store.get_mut(plugin_id) { + delegations.remove(grant_id); + if delegations.is_empty() { + store.remove(plugin_id); + } } }); } pub fn revoke_all_grants(plugin_id: &str) { with_store(|store| { - if let Some(delegations) = store.get_mut(plugin_id) { - for delegation in delegations.values_mut() { - delegation.revoked = true; + store.remove(plugin_id); + }); +} + +pub fn revoke_grant_everywhere(grant_id: &str) -> bool { + with_store(|store| { + let mut removed = false; + let mut empty_plugins = Vec::new(); + for (plugin_id, delegations) in store.iter_mut() { + if delegations.remove(grant_id).is_some() { + removed = true; + } + if delegations.is_empty() { + empty_plugins.push(plugin_id.clone()); } } - }); + for plugin_id in empty_plugins { + store.remove(&plugin_id); + } + removed + }) } pub fn is_delegated(plugin_id: &str, grant_id: &str) -> bool { with_store(|store| { store .get(plugin_id) - .and_then(|delegations| delegations.get(grant_id)) - .is_some_and(|delegation| !delegation.revoked) + .is_some_and(|delegations| delegations.contains_key(grant_id)) }) } @@ -106,9 +113,8 @@ pub fn list_delegated_grants(plugin_id: &str) -> Vec { let mut grant_ids = store .get(plugin_id) .into_iter() - .flat_map(|delegations| delegations.values()) - .filter(|delegation| !delegation.revoked) - .map(|delegation| delegation.grant_id.clone()) + .flat_map(|delegations| delegations.keys()) + .cloned() .collect::>(); grant_ids.sort(); grant_ids @@ -193,4 +199,16 @@ mod tests { assert!(grant_store::resolve_grant(&first).is_ok()); assert!(grant_store::resolve_grant(&second).is_ok()); } + + #[test] + fn revoke_grant_everywhere_prunes_empty_plugin_entries() { + let _guard = lock_grant_state(); + let grant_id = create_grant(); + + delegate_grant("acme.search", &grant_id).expect("delegate"); + assert!(revoke_grant_everywhere(&grant_id)); + + assert!(!is_delegated("acme.search", &grant_id)); + assert!(list_delegated_grants("acme.search").is_empty()); + } } diff --git a/crates/volt-plugin-host/src/engine.rs b/crates/volt-plugin-host/src/engine.rs index 469ef07..a44db8a 100644 --- a/crates/volt-plugin-host/src/engine.rs +++ b/crates/volt-plugin-host/src/engine.rs @@ -8,6 +8,7 @@ use boa_engine::module::{MapModuleLoader, Module}; use boa_engine::object::builtins::JsPromise; use boa_engine::{JsValue, Source, js_string}; use tokio::runtime::Builder as TokioRuntimeBuilder; +use tokio::task::yield_now; use crate::config::PluginConfig; use crate::ipc::{IpcMessage, MessageType}; @@ -235,7 +236,10 @@ impl PluginEngine { .map_err(|error| error.to_string())?; match promise.state() { - PromiseState::Pending => continue, + PromiseState::Pending => { + yield_now().await; + continue; + } PromiseState::Fulfilled(value) => return Ok(value), PromiseState::Rejected(error) => return Err(error.display().to_string()), } diff --git a/crates/volt-plugin-host/src/runtime_state.rs b/crates/volt-plugin-host/src/runtime_state.rs index 331e5be..d4aaf9c 100644 --- a/crates/volt-plugin-host/src/runtime_state.rs +++ b/crates/volt-plugin-host/src/runtime_state.rs @@ -4,14 +4,14 @@ use std::io::{self, BufReader}; use serde_json::Value; -use crate::config::DelegatedGrant; -use crate::config::PluginConfig; +use crate::config::{DelegatedGrant, HostIpcSettings, PluginConfig}; use crate::ipc::{IpcMessage, MessageType, read_message, write_message}; struct RuntimeState { plugin_id: String, manifest: Value, delegated_grants: Vec, + max_deferred_messages: usize, transport: Transport, } @@ -36,11 +36,13 @@ thread_local! { } pub fn configure_stdio(config: &PluginConfig) { + let host_ipc_settings = config.host_ipc_settings.clone().unwrap_or_default(); STATE.with(|state| { *state.borrow_mut() = Some(RuntimeState { plugin_id: config.plugin_id.clone(), manifest: config.manifest.clone(), delegated_grants: config.delegated_grants.clone(), + max_deferred_messages: max_deferred_messages(&host_ipc_settings), transport: Transport::StdIo { reader: BufReader::new(std::io::stdin()), writer: std::io::stdout(), @@ -157,7 +159,9 @@ pub fn send_request(method: impl Into, payload: Value) -> Result Result<(), String> { match self { - Self::StdIo { deferred, .. } => deferred.push_back(message), + Self::StdIo { deferred, .. } => { + if deferred.len() >= max_deferred_messages { + return Err(format!( + "host deferred message queue exceeded {} messages while waiting for a synchronous response", + max_deferred_messages + )); + } + deferred.push_back(message); + Ok(()) + } #[cfg(test)] - Self::Mock { deferred, .. } => deferred.push_back(message), + Self::Mock { deferred, .. } => { + if deferred.len() >= max_deferred_messages { + return Err(format!( + "host deferred message queue exceeded {} messages while waiting for a synchronous response", + max_deferred_messages + )); + } + deferred.push_back(message); + Ok(()) + } } } @@ -243,11 +265,13 @@ impl Transport { #[cfg(test)] pub(crate) fn configure_mock(config: &PluginConfig, inbound: Vec) { + let host_ipc_settings = config.host_ipc_settings.clone().unwrap_or_default(); STATE.with(|state| { *state.borrow_mut() = Some(RuntimeState { plugin_id: config.plugin_id.clone(), manifest: config.manifest.clone(), delegated_grants: config.delegated_grants.clone(), + max_deferred_messages: max_deferred_messages(&host_ipc_settings), transport: Transport::Mock { inbound: inbound.into(), outbound: Vec::new(), @@ -270,5 +294,9 @@ pub(crate) fn take_outbound() -> Vec { }) } +fn max_deferred_messages(settings: &HostIpcSettings) -> usize { + settings.max_queue_depth.max(1) as usize +} + #[cfg(test)] mod tests; diff --git a/crates/volt-plugin-host/src/runtime_state/tests.rs b/crates/volt-plugin-host/src/runtime_state/tests.rs index c1f8dd3..35f95c9 100644 --- a/crates/volt-plugin-host/src/runtime_state/tests.rs +++ b/crates/volt-plugin-host/src/runtime_state/tests.rs @@ -1,6 +1,6 @@ use serde_json::Value; -use crate::config::PluginConfig; +use crate::config::{HostIpcSettings, PluginConfig}; use crate::ipc::IpcMessage; use crate::runtime_state::{configure_mock, send_request, take_outbound}; @@ -16,6 +16,15 @@ fn test_config() -> PluginConfig { } } +fn test_config_with_queue_depth(max_queue_depth: u32) -> PluginConfig { + let mut config = test_config(); + config.host_ipc_settings = Some(HostIpcSettings { + max_queue_depth, + ..HostIpcSettings::default() + }); + config +} + #[test] fn send_request_writes_request_and_reads_response() { configure_mock( @@ -61,3 +70,19 @@ fn send_request_acks_heartbeat_while_waiting() { assert_eq!(outbound.len(), 2); assert_eq!(outbound[1].method, "heartbeat-ack"); } + +#[test] +fn send_request_rejects_when_deferred_queue_exceeds_limit() { + configure_mock( + &test_config_with_queue_depth(1), + vec![ + IpcMessage::signal("event-1", "plugin:event"), + IpcMessage::signal("event-2", "plugin:event"), + ], + ); + + let error = send_request("plugin:fs:exists", serde_json::json!({ "path": "cache" })) + .expect_err("error"); + + assert!(error.contains("deferred message queue exceeded 1 messages")); +} diff --git a/crates/volt-runner/src/ipc_bridge/response.rs b/crates/volt-runner/src/ipc_bridge/response.rs index 8c2f1d5..aba507e 100644 --- a/crates/volt-runner/src/ipc_bridge/response.rs +++ b/crates/volt-runner/src/ipc_bridge/response.rs @@ -2,6 +2,7 @@ use volt_core::command::{self, AppCommand}; use volt_core::ipc::{IPC_HANDLER_ERROR_CODE, IpcResponse, response_script}; pub(super) fn send_response_to_window(js_window_id: &str, response: IpcResponse) { + let request_id = response.id.clone(); let response_json = match serde_json::to_string(&response) { Ok(serialized) => serialized, Err(error) => { @@ -18,8 +19,15 @@ pub(super) fn send_response_to_window(js_window_id: &str, response: IpcResponse) }; let script = response_script(&response_json); - let _ = command::send_command(AppCommand::EvaluateScript { + if let Err(error) = command::send_command(AppCommand::EvaluateScript { js_id: js_window_id.to_string(), script, - }); + }) { + tracing::debug!( + window_id = %js_window_id, + request_id = %request_id, + error = %error, + "dropping IPC response because the target window is unavailable" + ); + } } diff --git a/crates/volt-runner/src/plugin_manager/discovery.rs b/crates/volt-runner/src/plugin_manager/discovery.rs index b201396..10da5da 100644 --- a/crates/volt-runner/src/plugin_manager/discovery.rs +++ b/crates/volt-runner/src/plugin_manager/discovery.rs @@ -262,6 +262,7 @@ impl PluginManager { registrations: PluginRegistrations::default(), delegated_grants: HashSet::new(), storage_reconciled: false, + spawn_cancelled: false, spawn_lock: Arc::new(Mutex::new(())), }) } diff --git a/crates/volt-runner/src/plugin_manager/host_api_access.rs b/crates/volt-runner/src/plugin_manager/host_api_access.rs index 1d85a1a..e800b0f 100644 --- a/crates/volt-runner/src/plugin_manager/host_api_access.rs +++ b/crates/volt-runner/src/plugin_manager/host_api_access.rs @@ -141,7 +141,10 @@ impl PluginManager { message: format!("grant '{grant_id}' is not delegated to plugin '{plugin_id}'"), }); } - volt_core::grant_store::resolve_grant(grant_id).map_err(fs_error) + volt_core::grant_store::resolve_grant(grant_id).map_err(|error| PluginRuntimeError { + code: PLUGIN_FS_ERROR_CODE.to_string(), + message: format!("grant '{grant_id}' is unavailable for plugin '{plugin_id}': {error}"), + }) } } @@ -198,10 +201,3 @@ fn access_registry_error(error: impl std::fmt::Display) -> PluginRuntimeError { message: error.to_string(), } } - -fn fs_error(error: impl std::fmt::Display) -> PluginRuntimeError { - PluginRuntimeError { - code: PLUGIN_FS_ERROR_CODE.to_string(), - message: error.to_string(), - } -} diff --git a/crates/volt-runner/src/plugin_manager/host_api_storage.rs b/crates/volt-runner/src/plugin_manager/host_api_storage.rs index 6c95984..f6c0368 100644 --- a/crates/volt-runner/src/plugin_manager/host_api_storage.rs +++ b/crates/volt-runner/src/plugin_manager/host_api_storage.rs @@ -186,7 +186,11 @@ fn write_bytes_atomic(root: &Path, relative_path: &str, data: &[u8]) -> Result<( let final_resolved = volt_core::fs::safe_resolve_for_create(root, relative_path) .map_err(|error| error.to_string())?; std::fs::write(&temp_resolved, data).map_err(|error| error.to_string())?; - replace_path_atomic(&temp_resolved, &final_resolved) + if let Err(error) = replace_path_atomic(&temp_resolved, &final_resolved) { + let _ = std::fs::remove_file(&temp_resolved); + return Err(error); + } + Ok(()) } fn hash_key(key: &str) -> String { diff --git a/crates/volt-runner/src/plugin_manager/lifecycle.rs b/crates/volt-runner/src/plugin_manager/lifecycle.rs index 3377e01..3096d1e 100644 --- a/crates/volt-runner/src/plugin_manager/lifecycle.rs +++ b/crates/volt-runner/src/plugin_manager/lifecycle.rs @@ -178,6 +178,7 @@ fn is_valid_transition(current: PluginState, next: PluginState) -> bool { | (PluginState::Terminated, PluginState::Spawning) | (PluginState::Failed, PluginState::Spawning) | (PluginState::Disabled, PluginState::Validated) + | (PluginState::Spawning, PluginState::Terminated) | (PluginState::Spawning, PluginState::Loaded) | (PluginState::Loaded, PluginState::Terminated) | (PluginState::Loaded, PluginState::Active) diff --git a/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs b/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs index 790aed1..de50198 100644 --- a/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs +++ b/crates/volt-runner/src/plugin_manager/lifecycle_bus.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::panic::{self, AssertUnwindSafe}; use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, MutexGuard}; use serde::Serialize; @@ -66,23 +66,15 @@ impl LifecycleBus { #[allow(dead_code)] pub(crate) fn off(&self, subscription_id: SubscriptionId) { - if let Ok(mut subscribers) = self.subscribers.lock() { - subscribers.remove(&subscription_id); - } + lock_subscribers(&self.subscribers).remove(&subscription_id); } pub(crate) fn emit(&self, event: PluginLifecycleEvent) { - let handlers = self - .subscribers - .lock() - .map(|subscribers| { - subscribers - .values() - .filter(|(topic, _)| topic_matches(*topic, &event)) - .map(|(_, handler)| handler.clone()) - .collect::>() - }) - .unwrap_or_default(); + let handlers = lock_subscribers(&self.subscribers) + .values() + .filter(|(topic, _)| topic_matches(*topic, &event)) + .map(|(_, handler)| handler.clone()) + .collect::>(); for handler in handlers { if let Err(payload) = panic::catch_unwind(AssertUnwindSafe(|| handler(&event))) { @@ -102,13 +94,23 @@ impl LifecycleBus { handler: Box, ) -> SubscriptionId { let id = SubscriptionId(self.next_id.fetch_add(1, Ordering::Relaxed)); - if let Ok(mut subscribers) = self.subscribers.lock() { - subscribers.insert(id, (topic, Arc::from(handler))); - } + lock_subscribers(&self.subscribers).insert(id, (topic, Arc::from(handler))); id } } +fn lock_subscribers( + subscribers: &Mutex>, +) -> MutexGuard<'_, HashMap> { + match subscribers.lock() { + Ok(guard) => guard, + Err(error) => { + tracing::warn!("plugin lifecycle bus subscriber registry was poisoned; recovering"); + error.into_inner() + } + } +} + fn panic_payload_message(payload: &Box) -> String { if let Some(message) = payload.downcast_ref::<&str>() { return (*message).to_string(); @@ -131,3 +133,53 @@ fn topic_matches(topic: LifecycleTopic, event: &PluginLifecycleEvent) -> bool { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn sample_event() -> PluginLifecycleEvent { + PluginLifecycleEvent { + plugin_id: "acme.search".to_string(), + previous_state: Some(PluginState::Loaded), + new_state: PluginState::Active, + timestamp: 1, + error: None, + } + } + + #[test] + fn subscribe_and_off_recover_after_mutex_poisoning() { + let bus = LifecycleBus::new(); + let subscribers = bus.subscribers.clone(); + let _ = std::panic::catch_unwind(|| { + let _guard = subscribers.lock().expect("subscribers"); + panic!("poison"); + }); + + let subscription = bus.on_lifecycle(Box::new(|_| {})); + assert!(lock_subscribers(&bus.subscribers).contains_key(&subscription)); + + bus.off(subscription); + assert!(!lock_subscribers(&bus.subscribers).contains_key(&subscription)); + } + + #[test] + fn emit_recover_after_mutex_poisoning() { + let bus = LifecycleBus::new(); + let seen = Arc::new(Mutex::new(Vec::new())); + let seen_for_handler = seen.clone(); + bus.on_lifecycle(Box::new(move |event| { + seen_for_handler.lock().expect("seen").push(event.new_state); + })); + + let subscribers = bus.subscribers.clone(); + let _ = std::panic::catch_unwind(|| { + let _guard = subscribers.lock().expect("subscribers"); + panic!("poison"); + }); + + bus.emit(sample_event()); + assert_eq!(*seen.lock().expect("seen"), vec![PluginState::Active]); + } +} diff --git a/crates/volt-runner/src/plugin_manager/mod.rs b/crates/volt-runner/src/plugin_manager/mod.rs index 1a0d1d2..468d17d 100644 --- a/crates/volt-runner/src/plugin_manager/mod.rs +++ b/crates/volt-runner/src/plugin_manager/mod.rs @@ -179,6 +179,7 @@ struct PluginRecord { registrations: PluginRegistrations, delegated_grants: HashSet, storage_reconciled: bool, + spawn_cancelled: bool, spawn_lock: Arc>, } diff --git a/crates/volt-runner/src/plugin_manager/runtime/lifecycle/shutdown.rs b/crates/volt-runner/src/plugin_manager/runtime/lifecycle/shutdown.rs index 979c521..e5326d1 100644 --- a/crates/volt-runner/src/plugin_manager/runtime/lifecycle/shutdown.rs +++ b/crates/volt-runner/src/plugin_manager/runtime/lifecycle/shutdown.rs @@ -13,7 +13,26 @@ impl PluginManager { else { return; }; - if !matches!( + if state == PluginState::Spawning { + crate::plugin_manager::host_api_helpers::clear_plugin_registrations_locked( + &mut registry, + plugin_id, + ); + let process = registry + .plugins + .get(plugin_id) + .and_then(|record| record.process.clone()); + let events = self + .transition_plugin_locked(&mut registry, plugin_id, PluginState::Terminated) + .map(|event| vec![event]) + .unwrap_or_default(); + if let Some(record) = registry.plugins.get_mut(plugin_id) { + record.process = None; + record.pending_requests = 0; + record.spawn_cancelled = true; + } + (process, state, events) + } else if !matches!( state, PluginState::Loaded | PluginState::Active @@ -28,23 +47,23 @@ impl PluginManager { record.process = None; } return; + } else { + let mut events = Vec::new(); + if matches!(state, PluginState::Active | PluginState::Running) + && let Ok(event) = self.transition_plugin_locked( + &mut registry, + plugin_id, + PluginState::Deactivating, + ) + { + events.push(event); + } + let process = registry + .plugins + .get(plugin_id) + .and_then(|record| record.process.clone()); + (process, state, events) } - - let mut events = Vec::new(); - if matches!(state, PluginState::Active | PluginState::Running) - && let Ok(event) = self.transition_plugin_locked( - &mut registry, - plugin_id, - PluginState::Deactivating, - ) - { - events.push(event); - } - let process = registry - .plugins - .get(plugin_id) - .and_then(|record| record.process.clone()); - (process, state, events) }; for event in pre_events { self.emit_lifecycle_event(event); @@ -53,7 +72,7 @@ impl PluginManager { let Some(process) = process else { return; }; - let result = if state == PluginState::Loaded { + let result = if matches!(state, PluginState::Loaded | PluginState::Spawning) { process.kill() } else { process.deactivate(self.deactivation_timeout()) @@ -70,6 +89,7 @@ impl PluginManager { if let Some(record) = registry.plugins.get_mut(plugin_id) { record.process = None; record.pending_requests = 0; + record.spawn_cancelled = false; } match result { Ok(()) => { diff --git a/crates/volt-runner/src/plugin_manager/runtime/lifecycle/spawn.rs b/crates/volt-runner/src/plugin_manager/runtime/lifecycle/spawn.rs index fb2a910..b9cd13f 100644 --- a/crates/volt-runner/src/plugin_manager/runtime/lifecycle/spawn.rs +++ b/crates/volt-runner/src/plugin_manager/runtime/lifecycle/spawn.rs @@ -78,6 +78,7 @@ impl PluginManager { manager.handle_plugin_message(&plugin_id_for_messages, message) })); self.register_process(plugin_id, process.clone(), now_ms()); + self.abort_cancelled_spawn(plugin_id, process.clone())?; if let Err(error) = process.wait_for_ready(self.activation_timeout()) { self.fail_plugin( @@ -90,6 +91,7 @@ impl PluginManager { let _ = process.kill(); return Err(unavailable_message(plugin_id, "failed before ready")); } + self.abort_cancelled_spawn(plugin_id, process.clone())?; self.transition_plugin(plugin_id, PluginState::Loaded)?; if mode == PluginStartupMode::LoadOnly { return Ok(process); @@ -190,8 +192,9 @@ impl PluginManager { let record = registry .plugins - .get(plugin_id) + .get_mut(plugin_id) .expect("plugin exists after state transition"); + record.spawn_cancelled = false; let data_root = record.data_root.clone().ok_or_else(|| PluginRuntimeError { code: PLUGIN_RUNTIME_ERROR_CODE.to_string(), message: format!("plugin '{plugin_id}' is missing a data root"), @@ -241,6 +244,34 @@ impl PluginManager { record.process = Some(process); } } + + fn abort_cancelled_spawn( + &self, + plugin_id: &str, + process: Arc, + ) -> Result<(), PluginRuntimeError> { + let cancelled = self + .inner + .registry + .lock() + .map_err(|_| registry_unavailable())? + .plugins + .get(plugin_id) + .map(|record| record.spawn_cancelled) + .ok_or_else(|| unavailable_plugin(plugin_id))?; + if !cancelled { + return Ok(()); + } + + let _ = process.kill(); + if let Ok(mut registry) = self.inner.registry.lock() + && let Some(record) = registry.plugins.get_mut(plugin_id) + { + record.process = None; + record.pending_requests = 0; + } + Err(unavailable_message(plugin_id, "spawn was cancelled")) + } } fn registry_unavailable() -> PluginRuntimeError { diff --git a/crates/volt-runner/src/plugin_manager/tests/activation.rs b/crates/volt-runner/src/plugin_manager/tests/activation.rs index 0dbf735..47e0253 100644 --- a/crates/volt-runner/src/plugin_manager/tests/activation.rs +++ b/crates/volt-runner/src/plugin_manager/tests/activation.rs @@ -251,3 +251,55 @@ fn pre_spawn_forces_startup_activation_after_window_ready_hook() { PluginState::Running ); } + +#[test] +fn deactivation_during_spawning_cancels_startup_and_terminates_plugin() { + let root = TempDir::new("spawn-cancel"); + write_manifest( + &root.join("plugins/acme.search/volt-plugin.json"), + "acme.search", + &["fs"], + ); + let plan = FakePlan { + ready: FakeOutcome::DelayOk(40), + ..FakePlan::default() + }; + let killed = plan.killed.clone(); + let manager = manager_with_factory( + RunnerPluginConfig { + enabled: vec!["acme.search".to_string()], + grants: BTreeMap::from([("acme.search".to_string(), vec!["fs".to_string()])]), + plugin_dirs: vec![root.join("plugins").display().to_string()], + ..RunnerPluginConfig::default() + }, + Arc::new(FakeProcessFactory::new(HashMap::from([( + "acme.search".to_string(), + plan, + )]))), + ); + register_ipc_handler(&manager, "acme.search", "ping"); + + let manager_for_request = manager.clone(); + let request = std::thread::spawn(move || { + manager_for_request.handle_ipc_request( + &IpcRequest { + id: "req-1".to_string(), + method: "plugin:acme.search:ping".to_string(), + args: Value::Null, + }, + Duration::from_millis(100), + ) + }); + + std::thread::sleep(Duration::from_millis(5)); + manager.deactivate_plugin("acme.search"); + + let response = request.join().expect("request thread").expect("response"); + assert!(response.result.is_none()); + assert!(response.error.is_some()); + assert!(killed.load(Ordering::Relaxed)); + + let snapshot = manager.get_plugin_state("acme.search").expect("plugin"); + assert_eq!(snapshot.current_state, PluginState::Terminated); + assert!(!snapshot.process_running); +} diff --git a/crates/volt-runner/src/plugin_manager/tests/lifecycle.rs b/crates/volt-runner/src/plugin_manager/tests/lifecycle.rs index 3623ca7..7d76ec2 100644 --- a/crates/volt-runner/src/plugin_manager/tests/lifecycle.rs +++ b/crates/volt-runner/src/plugin_manager/tests/lifecycle.rs @@ -37,6 +37,29 @@ fn state_machine_rejects_invalid_transitions() { assert!(lifecycle.transition(PluginState::Loaded).is_err()); } +#[test] +fn state_machine_only_allows_terminated_self_transition() { + let mut lifecycle = PluginLifecycle::new(); + lifecycle + .transition(PluginState::Discovered) + .expect("discovered"); + lifecycle + .transition(PluginState::Validated) + .expect("validated"); + assert!(lifecycle.transition(PluginState::Validated).is_err()); + + lifecycle + .transition(PluginState::Spawning) + .expect("spawning"); + lifecycle.transition(PluginState::Loaded).expect("loaded"); + lifecycle + .transition(PluginState::Terminated) + .expect("terminated"); + lifecycle + .transition(PluginState::Terminated) + .expect("terminated self-transition"); +} + #[test] fn get_states_returns_sorted_plugin_snapshots() { let root = TempDir::new("states"); @@ -166,3 +189,31 @@ fn state_snapshot_reports_active_registrations_and_grants() { assert_eq!(snapshot.active_registrations.ipc_handler_count, 1); assert_eq!(snapshot.delegated_grant_count, 1); } + +#[test] +fn disabled_plugins_ignore_process_exit_transitions() { + let root = TempDir::new("disabled-exit"); + write_manifest( + &root.join("plugins/acme.search/volt-plugin.json"), + "acme.search", + &["fs"], + ); + let manager = manager_with_factory( + RunnerPluginConfig { + enabled: Vec::new(), + plugin_dirs: vec![root.join("plugins").display().to_string()], + ..RunnerPluginConfig::default() + }, + Arc::new(FakeProcessFactory::new(HashMap::new())), + ); + + manager.handle_process_exit("acme.search", ProcessExitInfo { code: Some(0) }); + + assert_eq!( + manager + .get_plugin_state("acme.search") + .expect("plugin") + .current_state, + PluginState::Disabled + ); +} diff --git a/crates/volt-runner/src/plugin_manager/tests/process_support.rs b/crates/volt-runner/src/plugin_manager/tests/process_support.rs index dacf48b..a5bd0d0 100644 --- a/crates/volt-runner/src/plugin_manager/tests/process_support.rs +++ b/crates/volt-runner/src/plugin_manager/tests/process_support.rs @@ -45,6 +45,7 @@ impl Default for FakePlan { pub(super) enum FakeOutcome { #[default] Ok, + DelayOk(u64), Timeout, Error(&'static str), Crash(i32), @@ -114,6 +115,10 @@ impl PluginProcessHandle for FakeProcessHandle { fn wait_for_ready(&self, _timeout: Duration) -> Result<(), PluginRuntimeError> { match self.plan.lock().expect("plan").ready.clone() { FakeOutcome::Ok => Ok(()), + FakeOutcome::DelayOk(delay_ms) => { + std::thread::sleep(Duration::from_millis(delay_ms)); + Ok(()) + } FakeOutcome::Timeout => Err(PluginRuntimeError { code: "TIMEOUT".to_string(), message: "ready timeout".to_string(), @@ -135,6 +140,10 @@ impl PluginProcessHandle for FakeProcessHandle { fn activate(&self, _timeout: Duration) -> Result<(), PluginRuntimeError> { match self.plan.lock().expect("plan").activate.clone() { FakeOutcome::Ok => Ok(()), + FakeOutcome::DelayOk(delay_ms) => { + std::thread::sleep(Duration::from_millis(delay_ms)); + Ok(()) + } FakeOutcome::Timeout => Err(PluginRuntimeError { code: "TIMEOUT".to_string(), message: "activate timeout".to_string(), @@ -224,6 +233,10 @@ impl PluginProcessHandle for FakeProcessHandle { }; match outcome { FakeOutcome::Ok => Ok(()), + FakeOutcome::DelayOk(delay_ms) => { + std::thread::sleep(Duration::from_millis(delay_ms)); + Ok(()) + } FakeOutcome::Timeout => Err(PluginRuntimeError { code: PLUGIN_HEARTBEAT_TIMEOUT_CODE.to_string(), message: "heartbeat timeout".to_string(), @@ -248,6 +261,11 @@ impl PluginProcessHandle for FakeProcessHandle { self.notify_exit(0); Ok(()) } + FakeOutcome::DelayOk(delay_ms) => { + std::thread::sleep(Duration::from_millis(delay_ms)); + self.notify_exit(0); + Ok(()) + } FakeOutcome::Timeout => Err(PluginRuntimeError { code: "TIMEOUT".to_string(), message: "deactivate timeout".to_string(), diff --git a/crates/volt-runner/src/plugin_manager/watchdog.rs b/crates/volt-runner/src/plugin_manager/watchdog.rs index 73fab84..b8faef9 100644 --- a/crates/volt-runner/src/plugin_manager/watchdog.rs +++ b/crates/volt-runner/src/plugin_manager/watchdog.rs @@ -27,9 +27,8 @@ impl PluginManager { .get(plugin_id) .map(|record| record.lifecycle.current_state()) { - Some( - PluginState::Deactivating | PluginState::Terminated | PluginState::Disabled, - ) => self + Some(PluginState::Disabled) => Vec::new(), + Some(PluginState::Deactivating | PluginState::Terminated) => self .transition_plugin_locked(&mut registry, plugin_id, PluginState::Terminated) .map(|event| vec![event]) .unwrap_or_default(), From e6217715eb2b45b998a9c2bba3896776f2cd8450 Mon Sep 17 00:00:00 2001 From: MerhiOPS Date: Mon, 16 Mar 2026 14:41:50 +0200 Subject: [PATCH 4/4] fix(parity): tighten dev secure storage limits --- .../src/plugin_manager/host_api_storage.rs | 5 ++ .../src/plugin_manager/tests/activation.rs | 6 ++- .../plugin_manager/tests/process_support.rs | 13 ++++- .../plugin_manager/tests/request_runtime.rs | 8 ++-- docs/api/backend-runtime-modules.md | 1 + .../dev-backend-runtime-modules.test.ts | 47 +++++++++++++++++++ .../runtime-modules/volt-secure-storage.ts | 13 ++++- 7 files changed, 86 insertions(+), 7 deletions(-) diff --git a/crates/volt-runner/src/plugin_manager/host_api_storage.rs b/crates/volt-runner/src/plugin_manager/host_api_storage.rs index f6c0368..8bbb0c3 100644 --- a/crates/volt-runner/src/plugin_manager/host_api_storage.rs +++ b/crates/volt-runner/src/plugin_manager/host_api_storage.rs @@ -20,6 +20,11 @@ impl PluginManager { operation: &str, payload: &Value, ) -> Result { + // Plugin storage requests are serialized per plugin today: + // the host reads one plugin message at a time and the plugin-side + // request API waits synchronously for the reply before sending the + // next storage operation. If that transport model changes, this code + // will need an explicit per-plugin storage lock. let (storage_root, should_reconcile) = self.prepare_storage_root(plugin_id)?; let mut storage = PluginStorage::open(&storage_root, should_reconcile).map_err(storage_error)?; diff --git a/crates/volt-runner/src/plugin_manager/tests/activation.rs b/crates/volt-runner/src/plugin_manager/tests/activation.rs index 47e0253..f1b082e 100644 --- a/crates/volt-runner/src/plugin_manager/tests/activation.rs +++ b/crates/volt-runner/src/plugin_manager/tests/activation.rs @@ -8,7 +8,9 @@ use volt_core::ipc::IpcRequest; use super::super::*; use super::fs_support::{TempDir, write_manifest}; -use super::process_support::{FakeOutcome, FakePlan, FakeProcessFactory, FakeRequestOutcome}; +use super::process_support::{ + FakeOutcome, FakePlan, FakeProcessFactory, FakeRequestOutcome, wait_for_flag, +}; use super::shared::{manager_with_factory, register_ipc_handler}; use crate::runner::config::{RunnerPluginConfig, RunnerPluginSpawning}; @@ -297,7 +299,7 @@ fn deactivation_during_spawning_cancels_startup_and_terminates_plugin() { let response = request.join().expect("request thread").expect("response"); assert!(response.result.is_none()); assert!(response.error.is_some()); - assert!(killed.load(Ordering::Relaxed)); + assert!(wait_for_flag(killed.as_ref(), Duration::from_millis(200))); let snapshot = manager.get_plugin_state("acme.search").expect("plugin"); assert_eq!(snapshot.current_state, PluginState::Terminated); diff --git a/crates/volt-runner/src/plugin_manager/tests/process_support.rs b/crates/volt-runner/src/plugin_manager/tests/process_support.rs index a5bd0d0..52f8e61 100644 --- a/crates/volt-runner/src/plugin_manager/tests/process_support.rs +++ b/crates/volt-runner/src/plugin_manager/tests/process_support.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; -use std::time::Duration; +use std::time::{Duration, Instant}; use serde_json::Value; @@ -306,3 +306,14 @@ impl PluginProcessHandle for FakeProcessHandle { None } } + +pub(super) fn wait_for_flag(flag: &AtomicBool, timeout: Duration) -> bool { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if flag.load(Ordering::Relaxed) { + return true; + } + std::thread::sleep(Duration::from_millis(5)); + } + flag.load(Ordering::Relaxed) +} diff --git a/crates/volt-runner/src/plugin_manager/tests/request_runtime.rs b/crates/volt-runner/src/plugin_manager/tests/request_runtime.rs index bd35302..6654270 100644 --- a/crates/volt-runner/src/plugin_manager/tests/request_runtime.rs +++ b/crates/volt-runner/src/plugin_manager/tests/request_runtime.rs @@ -1,6 +1,6 @@ use std::collections::{BTreeMap, HashMap}; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::AtomicBool; use std::thread; use std::time::Duration; @@ -9,7 +9,9 @@ use volt_core::ipc::IpcRequest; use super::super::*; use super::fs_support::{TempDir, write_manifest}; -use super::process_support::{FakeOutcome, FakePlan, FakeProcessFactory, FakeRequestOutcome}; +use super::process_support::{ + FakeOutcome, FakePlan, FakeProcessFactory, FakeRequestOutcome, wait_for_flag, +}; use super::shared::{manager_with_factory, register_ipc_handler}; use crate::runner::config::{RunnerPluginConfig, RunnerPluginLimits}; @@ -162,7 +164,7 @@ fn watchdog_kills_after_two_missed_heartbeats() { ); thread::sleep(Duration::from_millis(60)); - assert!(killed.load(Ordering::Relaxed)); + assert!(wait_for_flag(killed.as_ref(), Duration::from_millis(200))); assert_eq!( manager .get_plugin_state("acme.search") diff --git a/docs/api/backend-runtime-modules.md b/docs/api/backend-runtime-modules.md index d4afd27..ea5bdfb 100644 --- a/docs/api/backend-runtime-modules.md +++ b/docs/api/backend-runtime-modules.md @@ -101,6 +101,7 @@ Key-value secret storage for backend code. - key must be a non-empty string after trim - key length must be at most 256 characters +- value length must be at most 8192 bytes ### Example diff --git a/packages/volt-cli/src/__tests__/dev-backend-runtime-modules.test.ts b/packages/volt-cli/src/__tests__/dev-backend-runtime-modules.test.ts index e15e357..b00e3f3 100644 --- a/packages/volt-cli/src/__tests__/dev-backend-runtime-modules.test.ts +++ b/packages/volt-cli/src/__tests__/dev-backend-runtime-modules.test.ts @@ -285,4 +285,51 @@ describe('dev backend runtime modules', () => { }, }); }); + + it('enforces the secureStorage value size limit in dev mode', async () => { + const project = createTempProject({ + 'backend.ts': ` + import { ipcMain } from 'volt:ipc'; + import * as secureStorage from 'volt:secureStorage'; + + ipcMain.handle('dev-backend:secure-storage-limit', async () => { + return secureStorage + .set('token', 'x'.repeat(8193)) + .then(() => 'unexpected') + .catch((error) => String(error)); + }); + `, + 'tsconfig.json': JSON.stringify({ + compilerOptions: { + target: 'ES2022', + module: 'ESNext', + moduleResolution: 'bundler', + strict: true, + }, + }), + }); + cleanups.push(project.cleanup); + + configureRuntimeModuleState({ + projectRoot: project.rootDir, + defaultWindowId: 'window-dev', + nativeRuntime: { windowEvalScript: vi.fn() }, + permissions: ['secureStorage'], + }); + + const backendLoadState = await loadBackendEntrypointForDev(project.rootDir, './backend.ts'); + cleanups.push(backendLoadState.dispose); + + const response = await ipcMain.processRequest( + 'req-secure-storage-limit', + 'dev-backend:secure-storage-limit', + null, + { timeoutMs: 200 }, + ); + expect(response).toEqual({ + id: 'req-secure-storage-limit', + result: + 'Error: [volt:secureStorage] Secure storage value length must be <= 8192 bytes.', + }); + }); }); diff --git a/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts b/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts index e5ac2e8..327f400 100644 --- a/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts +++ b/packages/volt-cli/src/commands/dev/runtime-modules/volt-secure-storage.ts @@ -3,6 +3,7 @@ import { devModuleError, ensureDevPermission, resolveProjectScopedPath } from '. const storage = new Map(); let loaded = false; +const MAX_VALUE_LENGTH = 8192; function storageFilePath(): string { return resolveProjectScopedPath('storage.json', 'secure-storage'); @@ -48,10 +49,20 @@ function normalizeKey(key: string): string { return normalized; } +function normalizeValue(value: string): string { + if (Buffer.byteLength(value, 'utf8') > MAX_VALUE_LENGTH) { + throw devModuleError( + 'secureStorage', + `Secure storage value length must be <= ${MAX_VALUE_LENGTH} bytes.`, + ); + } + return value; +} + export async function set(key: string, value: string): Promise { ensureDevPermission('secureStorage', 'secureStorage.set()'); loadStorage(); - storage.set(normalizeKey(key), value); + storage.set(normalizeKey(key), normalizeValue(value)); persistStorage(); }