From d7590f6834f15462e6942792e1a5f8df86efaaa8 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 16 Apr 2026 15:05:19 -0500 Subject: [PATCH 1/2] fix(python): reject external handler re-entry --- crates/bashkit-python/bashkit/_bashkit.pyi | 3 + crates/bashkit-python/src/lib.rs | 130 ++++++++++++++++-- .../tests/_bashkit_categories.py | 49 +++++++ .../tests/test_error_handling.py | 2 + 4 files changed, 170 insertions(+), 14 deletions(-) diff --git a/crates/bashkit-python/bashkit/_bashkit.pyi b/crates/bashkit-python/bashkit/_bashkit.pyi index 1f3f5b4f..13f2c04e 100644 --- a/crates/bashkit-python/bashkit/_bashkit.pyi +++ b/crates/bashkit-python/bashkit/_bashkit.pyi @@ -372,6 +372,9 @@ class Bash: python: Enable embedded Python (``python3`` builtin). external_functions: Function names callable from Python code. external_handler: Async callback for external function calls. + The callback must not call back into the same ``Bash`` instance + via live methods like ``read_file()``, ``fs()``, or + ``execute()``; those re-entrant calls are rejected. files: Dict mapping VFS paths to file contents or lazy callables. mounts: List of real host directory mount configs. custom_builtins: Constructor-time Python callbacks exposed as diff --git a/crates/bashkit-python/src/lib.rs b/crates/bashkit-python/src/lib.rs index 99811366..01f70dbf 100644 --- a/crates/bashkit-python/src/lib.rs +++ b/crates/bashkit-python/src/lib.rs @@ -24,7 +24,7 @@ use pyo3_async_runtimes::tokio::future_into_py; use std::collections::HashMap; use std::future::Future; use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex as StdMutex, RwLock}; use std::time::{SystemTime, UNIX_EPOCH}; use tokio::runtime::Runtime; @@ -36,6 +36,7 @@ use tokio::sync::{Mutex, oneshot}; /// Convert serde_json::Value → Py const MAX_NESTING_DEPTH: usize = 64; +const EXTERNAL_HANDLER_REENTRY_ERROR: &str = "external_handler cannot re-enter the same Bash instance; use handler inputs or another Bash instance for live access"; fn json_to_py(py: Python<'_>, val: &serde_json::Value) -> PyResult> { json_to_py_inner(py, val, 0) @@ -502,21 +503,56 @@ fn dir_entry_to_pydict(py: Python<'_>, entry: &FsDirEntry) -> PyResult #[derive(Clone)] enum FileSystemHandle { Static(Arc), - Live(Arc>), + Live { + inner: Arc>, + external_handler_reentry_depth: Option>, + }, } impl FileSystemHandle { - async fn resolve(&self) -> Arc { + async fn resolve(&self) -> PyResult> { match self { - Self::Static(fs) => Arc::clone(fs), - Self::Live(inner) => { + Self::Static(fs) => Ok(Arc::clone(fs)), + Self::Live { + inner, + external_handler_reentry_depth, + } => { + reject_external_handler_reentry_depth(external_handler_reentry_depth.as_ref())?; let bash = inner.lock().await; - bash.fs() + Ok(bash.fs()) } } } } +fn reject_external_handler_reentry_depth( + external_handler_reentry_depth: Option<&Arc>, +) -> PyResult<()> { + if let Some(depth) = external_handler_reentry_depth + && depth.load(Ordering::Relaxed) > 0 + { + return Err(PyRuntimeError::new_err(EXTERNAL_HANDLER_REENTRY_ERROR)); + } + Ok(()) +} + +struct ExternalHandlerReentryScope { + depth: Arc, +} + +impl ExternalHandlerReentryScope { + fn enter(depth: Arc) -> Self { + depth.fetch_add(1, Ordering::Relaxed); + Self { depth } + } +} + +impl Drop for ExternalHandlerReentryScope { + fn drop(&mut self) { + self.depth.fetch_sub(1, Ordering::Relaxed); + } +} + fn with_live_fs(rt: &Arc, inner: &Arc>, f: F) -> PyResult where F: FnOnce(Arc) -> Fut, @@ -873,7 +909,24 @@ impl PyFileSystem { fn from_live(inner: Arc>, rt: Arc) -> Self { Self { - inner: FileSystemHandle::Live(inner), + inner: FileSystemHandle::Live { + inner, + external_handler_reentry_depth: None, + }, + rt, + } + } + + fn from_live_with_reentry_guard( + inner: Arc>, + rt: Arc, + external_handler_reentry_depth: Arc, + ) -> Self { + Self { + inner: FileSystemHandle::Live { + inner, + external_handler_reentry_depth: Some(external_handler_reentry_depth), + }, rt, } } @@ -885,7 +938,7 @@ impl PyFileSystem { { let inner = self.inner.clone(); self.rt.block_on(async move { - let fs = inner.resolve().await; + let fs = inner.resolve().await?; f(fs).await }) } @@ -2182,10 +2235,18 @@ async fn exec_bash_with_optional_output( /// /// The handler converts MontyObject args/kwargs to Python objects, calls the /// async handler coroutine, awaits it, and converts the result back. -fn make_external_handler(py_handler: Py) -> PythonExternalFnHandler { +// Decision: reject same-instance live Bash access from external_handler. +// Releasing the interpreter mutex during Python callbacks would widen the +// execution model; a targeted guard keeps the failure explicit and local. +fn make_external_handler( + py_handler: Py, + external_handler_reentry_depth: Arc, +) -> PythonExternalFnHandler { Arc::new(move |fn_name, args, kwargs| { let py_handler = Python::attach(|py| py_handler.clone_ref(py)); + let external_handler_reentry_depth = external_handler_reentry_depth.clone(); Box::pin(async move { + let _reentry_scope = ExternalHandlerReentryScope::enter(external_handler_reentry_depth); let fut = Python::attach(|py| { let py_args = args .iter() @@ -2232,6 +2293,7 @@ fn apply_python_config( python: bool, fn_names: Vec, handler: Option>, + external_handler_reentry_depth: Arc, ) -> bashkit::BashBuilder { // By construction, handler.is_some() implies python=true (validated in new()). match (python, handler) { @@ -2239,7 +2301,7 @@ fn apply_python_config( builder = builder.python_with_external_handler( PythonLimits::default(), fn_names, - make_external_handler(h), + make_external_handler(h, external_handler_reentry_depth), ); } (true, None) => { @@ -2281,6 +2343,7 @@ pub struct PyBash { external_functions: Vec, /// Async Python callable invoked when Monty calls an external function. external_handler: Option>, + external_handler_reentry_depth: Arc, custom_builtins: Vec, builtin_engine: Arc, files: Vec, @@ -2292,6 +2355,10 @@ pub struct PyBash { } impl PyBash { + fn reject_external_handler_reentry(&self) -> PyResult<()> { + reject_external_handler_reentry_depth(Some(&self.external_handler_reentry_depth)) + } + fn build_live_builder(&self, py: Python<'_>) -> PyResult { let mut builder = Bash::builder(); @@ -2325,6 +2392,7 @@ impl PyBash { self.python, self.external_functions.clone(), handler_clone, + self.external_handler_reentry_depth.clone(), ); let files = clone_file_mounts(py, &self.files); let builder = apply_fs_config(builder, &files, &self.real_mounts)?; @@ -2424,7 +2492,14 @@ impl PyBash { } } let handler_for_build = external_handler.as_ref().map(|h| h.clone_ref(py)); - builder = apply_python_config(builder, python, fn_names, handler_for_build); + let external_handler_reentry_depth = Arc::new(AtomicUsize::new(0)); + builder = apply_python_config( + builder, + python, + fn_names, + handler_for_build, + external_handler_reentry_depth.clone(), + ); builder = apply_fs_config(builder, &files, &real_mounts)?; let builtin_engine = PyCallbackEngine::new(py)?; builder = apply_custom_builtins_to_builder(py, builder, &custom_builtins); @@ -2443,6 +2518,7 @@ impl PyBash { python, external_functions: external_functions.unwrap_or_default(), external_handler, + external_handler_reentry_depth, custom_builtins, builtin_engine, files, @@ -2489,6 +2565,7 @@ impl PyBash { commands: String, on_output: Option>, ) -> PyResult> { + self.reject_external_handler_reentry()?; let builtin_session = capture_custom_builtin_session(py, &self.custom_builtins, &self.builtin_engine, true)?; let on_output = prepare_output_handler(py, on_output)?; @@ -2585,6 +2662,7 @@ impl PyBash { commands: String, on_output: Option>, ) -> PyResult> { + self.reject_external_handler_reentry()?; let builtin_session = capture_custom_builtin_session(py, &self.custom_builtins, &self.builtin_engine, true)?; let on_output = prepare_output_handler(py, on_output)?; @@ -2611,6 +2689,7 @@ impl PyBash { /// python mode, external function handler, and custom builtins. /// Releases GIL before blocking on tokio to prevent deadlock. fn reset(&self, py: Python<'_>) -> PyResult<()> { + self.reject_external_handler_reentry()?; replace_live_bash_with_builder( py, &self.rt, @@ -2627,17 +2706,20 @@ impl PyBash { py: Python<'py>, exclude_filesystem: bool, ) -> PyResult> { + self.reject_external_handler_reentry()?; let bytes = snapshot_live_bash(py, &self.rt, &self.inner, exclude_filesystem)?; Ok(PyBytes::new(py, &bytes)) } /// Capture a read-only shell-state snapshot for prompt rendering and inspection. fn shell_state(&self, py: Python<'_>) -> PyResult { + self.reject_external_handler_reentry()?; capture_shell_state(py, &self.rt, &self.inner) } /// Restore interpreter state from bytes previously produced by `snapshot()`. fn restore_snapshot(&self, py: Python<'_>, data: Vec) -> PyResult<()> { + self.reject_external_handler_reentry()?; restore_live_bash(py, &self.rt, &self.inner, data) } @@ -2695,49 +2777,60 @@ impl PyBash { } fn read_file(&self, py: Python<'_>, path: String) -> PyResult { + self.reject_external_handler_reentry()?; py.detach(|| read_text_via_live_fs(&self.rt, &self.inner, path)) } fn write_file(&self, py: Python<'_>, path: String, content: String) -> PyResult<()> { + self.reject_external_handler_reentry()?; py.detach(|| write_text_via_live_fs(&self.rt, &self.inner, path, content)) } fn append_file(&self, py: Python<'_>, path: String, content: String) -> PyResult<()> { + self.reject_external_handler_reentry()?; py.detach(|| append_text_via_live_fs(&self.rt, &self.inner, path, content)) } #[pyo3(signature = (path, recursive=false))] fn mkdir(&self, py: Python<'_>, path: String, recursive: bool) -> PyResult<()> { + self.reject_external_handler_reentry()?; py.detach(|| mkdir_via_live_fs(&self.rt, &self.inner, path, recursive)) } fn exists(&self, py: Python<'_>, path: String) -> PyResult { + self.reject_external_handler_reentry()?; py.detach(|| exists_via_live_fs(&self.rt, &self.inner, path)) } #[pyo3(signature = (path, recursive=false))] fn remove(&self, py: Python<'_>, path: String, recursive: bool) -> PyResult<()> { + self.reject_external_handler_reentry()?; py.detach(|| remove_via_live_fs(&self.rt, &self.inner, path, recursive)) } fn stat(&self, py: Python<'_>, path: String) -> PyResult> { + self.reject_external_handler_reentry()?; let metadata = py.detach(|| stat_via_live_fs(&self.rt, &self.inner, path))?; metadata_to_pydict(py, &metadata) } fn chmod(&self, py: Python<'_>, path: String, mode: u32) -> PyResult<()> { + self.reject_external_handler_reentry()?; py.detach(|| chmod_via_live_fs(&self.rt, &self.inner, path, mode)) } fn symlink(&self, py: Python<'_>, target: String, link: String) -> PyResult<()> { + self.reject_external_handler_reentry()?; py.detach(|| symlink_via_live_fs(&self.rt, &self.inner, target, link)) } fn read_link(&self, py: Python<'_>, path: String) -> PyResult { + self.reject_external_handler_reentry()?; py.detach(|| read_link_via_live_fs(&self.rt, &self.inner, path)) } fn read_dir(&self, py: Python<'_>, path: String) -> PyResult> { + self.reject_external_handler_reentry()?; let entries = py.detach(|| read_dir_via_live_fs(&self.rt, &self.inner, path))?; let items: Vec> = entries .iter() @@ -2748,6 +2841,7 @@ impl PyBash { #[pyo3(signature = (path=".".to_string()))] fn ls(&self, py: Python<'_>, path: String) -> PyResult> { + self.reject_external_handler_reentry()?; let names = match py.detach(|| read_dir_via_live_fs(&self.rt, &self.inner, path)) { Ok(entries) => entries .into_iter() @@ -2759,6 +2853,7 @@ impl PyBash { } fn glob(&self, py: Python<'_>, pattern: String) -> PyResult> { + self.reject_external_handler_reentry()?; let matches = py.detach(|| -> PyResult> { Ok(glob_via_bash(&self.rt, &self.inner, pattern)) })?; @@ -2772,19 +2867,25 @@ impl PyBash { /// batch reads where consistency isn't needed, prefer reading files via /// `execute_sync("cat ...")`. fn fs(&self, py: Python<'_>) -> PyResult> { + self.reject_external_handler_reentry()?; Py::new( py, - PyFileSystem::from_live(self.inner.clone(), self.rt.clone()), + PyFileSystem::from_live_with_reentry_guard( + self.inner.clone(), + self.rt.clone(), + self.external_handler_reentry_depth.clone(), + ), ) } /// Mount a filesystem at `vfs_path` without rebuilding the interpreter. fn mount(&self, py: Python<'_>, vfs_path: String, fs: PyRef<'_, PyFileSystem>) -> PyResult<()> { + self.reject_external_handler_reentry()?; let inner = self.inner.clone(); let source = fs.inner.clone(); py.detach(|| { self.rt.block_on(async move { - let mounted_fs = source.resolve().await; + let mounted_fs = source.resolve().await?; let bash = inner.lock().await; bash.mount(Path::new(&vfs_path), mounted_fs) .map_err(|e| PyRuntimeError::new_err(e.to_string())) @@ -2794,6 +2895,7 @@ impl PyBash { /// Unmount a live filesystem without rebuilding the interpreter. fn unmount(&self, py: Python<'_>, vfs_path: String) -> PyResult<()> { + self.reject_external_handler_reentry()?; let inner = self.inner.clone(); py.detach(|| { self.rt.block_on(async move { @@ -3316,7 +3418,7 @@ impl BashTool { let source = fs.inner.clone(); py.detach(|| { self.rt.block_on(async move { - let mounted_fs = source.resolve().await; + let mounted_fs = source.resolve().await?; let bash = inner.lock().await; bash.mount(Path::new(&vfs_path), mounted_fs) .map_err(|e| PyRuntimeError::new_err(e.to_string()))?; diff --git a/crates/bashkit-python/tests/_bashkit_categories.py b/crates/bashkit-python/tests/_bashkit_categories.py index 50de44cc..55cc2e5f 100644 --- a/crates/bashkit-python/tests/_bashkit_categories.py +++ b/crates/bashkit-python/tests/_bashkit_categories.py @@ -1703,6 +1703,55 @@ async def handler(fn_name: str, args: list, kwargs: dict): assert "service unavailable" in r.stderr or "service unavailable" in (r.error or "") +@pytest.mark.asyncio +async def test_bash_external_handler_reentrant_read_file_rejected(): + """Same-instance live VFS access from external_handler must fail fast.""" + + bash = None + + async def handler(fn_name: str, args: list, kwargs: dict): + assert fn_name == "read__memory" + return bash.read_file("/workspace/memory.md") + + bash = Bash( + python=True, + external_functions=["read__memory"], + external_handler=handler, + files={"/workspace/memory.md": "hello from memory\n"}, + ) + r = await asyncio.wait_for( + bash.execute('python3 -c "print(read__memory())"'), + timeout=1, + ) + assert r.exit_code != 0 + assert "external_handler cannot re-enter the same Bash instance" in (r.stderr + (r.error or "")) + + +@pytest.mark.asyncio +async def test_bash_external_handler_reentrant_fs_handle_rejected(): + """Live fs handles from the same Bash instance must also fail fast.""" + + bash = None + + async def handler(fn_name: str, args: list, kwargs: dict): + assert fn_name == "read__memory" + fs = bash.fs() + return fs.read_file("/workspace/memory.md").decode() + + bash = Bash( + python=True, + external_functions=["read__memory"], + external_handler=handler, + files={"/workspace/memory.md": "hello from memory\n"}, + ) + r = await asyncio.wait_for( + bash.execute('python3 -c "print(read__memory())"'), + timeout=1, + ) + assert r.exit_code != 0 + assert "external_handler cannot re-enter the same Bash instance" in (r.stderr + (r.error or "")) + + @pytest.mark.asyncio async def test_bash_reset_preserves_python_and_handler(): """reset() must preserve python=True and external_handler config.""" diff --git a/crates/bashkit-python/tests/test_error_handling.py b/crates/bashkit-python/tests/test_error_handling.py index 99483c6c..437f7eba 100644 --- a/crates/bashkit-python/tests/test_error_handling.py +++ b/crates/bashkit-python/tests/test_error_handling.py @@ -29,6 +29,8 @@ "test_bashtool_pre_exec_error_in_stderr", "test_bash_pre_exec_error_in_stderr_async", "test_bash_external_handler_error_propagates", + "test_bash_external_handler_reentrant_read_file_rejected", + "test_bash_external_handler_reentrant_fs_handle_rejected", "test_bash_external_functions_without_handler_raises", "test_bash_non_callable_handler_raises", "test_bash_external_handler_requires_python_true", From 5011591f36ef7f640b771337449ddcffc2e6b238 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 16 Apr 2026 15:28:25 -0500 Subject: [PATCH 2/2] fix(js): reject onOutput re-entry --- .../runtime-compat/streaming-output.test.mjs | 45 +++++++++++- crates/bashkit-js/src/lib.rs | 70 ++++++++++++++++--- crates/bashkit-js/wrapper.ts | 4 +- 3 files changed, 106 insertions(+), 13 deletions(-) diff --git a/crates/bashkit-js/__test__/runtime-compat/streaming-output.test.mjs b/crates/bashkit-js/__test__/runtime-compat/streaming-output.test.mjs index 50d6c480..a931f8d9 100644 --- a/crates/bashkit-js/__test__/runtime-compat/streaming-output.test.mjs +++ b/crates/bashkit-js/__test__/runtime-compat/streaming-output.test.mjs @@ -24,8 +24,8 @@ function assertChunksMatchResult(result, chunks) { } for (const [label, create] of [ - ["Bash", () => new Bash()], - ["BashTool", () => new BashTool()], + ["Bash", (options) => new Bash(options)], + ["BashTool", (options) => new BashTool(options)], ]) { describe(`${label} streaming output`, () => { it("sync rejects Promise-returning onOutput", () => { @@ -147,6 +147,24 @@ for (const [label, create] of [ assert.equal(result.stdout, "after-error\n"); }); + it("sync rejects same-instance execute from onOutput", () => { + const shell = create(); + + assert.throws( + () => + shell.executeSync(SCRIPT, { + onOutput() { + shell.executeSync("echo nested"); + }, + }), + /onOutput cannot re-enter the same Bash instance/, + ); + + const result = shell.executeSync("echo after-reentry"); + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, "after-reentry\n"); + }); + it("sync callback errors do not clear future explicit cancel", () => { const shell = create(); @@ -205,6 +223,29 @@ for (const [label, create] of [ assert.equal(result.stdout, ""); }); + it("async rejects same-instance fs handle from onOutput", async () => { + const shell = create({ + files: { + "/workspace/memory.md": "hello\n", + }, + }); + const fs = shell.fs(); + + await assert.rejects( + () => + shell.execute(SCRIPT, { + onOutput() { + fs.readFile("/workspace/memory.md"); + }, + }), + /onOutput cannot re-enter the same Bash instance/, + ); + + const result = await shell.execute("cat /workspace/memory.md"); + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, "hello\n"); + }); + it("async rejects async onOutput", async () => { const shell = create(); diff --git a/crates/bashkit-js/src/lib.rs b/crates/bashkit-js/src/lib.rs index 1f8b93dc..00ef0a3b 100644 --- a/crates/bashkit-js/src/lib.rs +++ b/crates/bashkit-js/src/lib.rs @@ -25,7 +25,7 @@ use napi_derive::napi; use std::collections::HashMap; use std::path::Path; use std::pin::Pin; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex as StdMutex}; use tokio::sync::Mutex; @@ -56,6 +56,34 @@ fn callback_semaphore() -> &'static tokio::sync::Semaphore { SEM.get_or_init(|| tokio::sync::Semaphore::new(MAX_CONCURRENT_TOOL_CALLBACKS)) } +// Decision: reject same-instance onOutput re-entry at the binding boundary so +// sync paths fail with a JS error instead of deadlocking or panicking. +const ON_OUTPUT_REENTRY_ERROR: &str = "onOutput cannot re-enter the same Bash instance; use collected output or another Bash instance for live access"; + +struct OnOutputReentryScope { + depth: Arc, +} + +impl OnOutputReentryScope { + fn enter(depth: Arc) -> Self { + depth.fetch_add(1, Ordering::SeqCst); + Self { depth } + } +} + +impl Drop for OnOutputReentryScope { + fn drop(&mut self) { + self.depth.fetch_sub(1, Ordering::SeqCst); + } +} + +fn reject_on_output_reentry(state: &Arc) -> napi::Result<()> { + if state.on_output_reentry_depth.load(Ordering::SeqCst) > 0 { + return Err(napi::Error::from_reason(ON_OUTPUT_REENTRY_ERROR)); + } + Ok(()) +} + // ============================================================================ // MontyObject <-> JSON conversion // ============================================================================ @@ -483,6 +511,7 @@ fn build_sync_output_callback( cancelled: Arc, callback_requested_cancel: Arc, callback_error: Arc>>, + on_output_reentry_depth: Arc, ) -> OutputCallback { Box::new(move |stdout_chunk, stderr_chunk| { let has_error = callback_error @@ -507,6 +536,7 @@ fn build_sync_output_callback( } }; + let _reentry_scope = OnOutputReentryScope::enter(on_output_reentry_depth.clone()); match callback.call((stdout_chunk.to_string(), stderr_chunk.to_string())) { Ok(Some(err)) => { record_callback_error( @@ -533,6 +563,7 @@ fn build_async_output_callback( tsfn: Arc, cancelled: Arc, callback_requested_cancel: Arc, + on_output_reentry_depth: Arc, ) -> (OutputCallback, Arc>>) { let callback_error = Arc::new(StdMutex::new(None)); let callback_error_output = callback_error.clone(); @@ -551,12 +582,14 @@ fn build_async_output_callback( let stdout = stdout_chunk.to_string(); let stderr = stderr_chunk.to_string(); let tsfn = tsfn.clone(); + let on_output_reentry_depth = on_output_reentry_depth.clone(); let (tx, rx) = std::sync::mpsc::channel(); // OutputCallback in core bashkit is synchronous. Dispatch onto the // shared callback runtime, then block until JS finishes so callback // errors abort execution immediately and chunk ordering stays stable. callback_runtime().spawn(async move { + let _reentry_scope = OnOutputReentryScope::enter(on_output_reentry_depth); let result: Result, String> = tsfn .call_async((stdout, stderr)) .await @@ -733,6 +766,7 @@ struct SharedState { inner: Mutex, rt: Mutex, cancelled: Arc, + on_output_reentry_depth: Arc, username: Option, hostname: Option, max_commands: Option, @@ -768,10 +802,14 @@ type ExternalHandlerArc = Arc< /// Clone `Arc`, then use the runtime to block on a future that /// captures only the cloned Arc. This avoids holding raw `&self` across /// `block_on` boundaries. -fn block_on_with(state: &Arc, f: impl FnOnce(Arc) -> Fut) -> T +fn block_on_with( + state: &Arc, + f: impl FnOnce(Arc) -> Fut, +) -> napi::Result where - Fut: std::future::Future, + Fut: std::future::Future>, { + reject_on_output_reentry(state)?; let s = state.clone(); let rt_guard = s.rt.blocking_lock(); let s2 = state.clone(); @@ -829,6 +867,7 @@ impl Bash { cancelled.clone(), callback_requested_cancel.clone(), callback_error.clone(), + s.on_output_reentry_depth.clone(), ); execute_rust_bash( &mut bash, @@ -848,6 +887,7 @@ impl Bash { /// Execute bash commands asynchronously, returning a Promise. #[napi] pub async fn execute(&self, commands: String) -> napi::Result { + reject_on_output_reentry(&self.state)?; let s = self.state.clone(); let mut bash = s.inner.lock().await; execute_rust_bash(&mut bash, &commands, None, None, None, None).await @@ -862,12 +902,14 @@ impl Bash { commands: String, on_output: napi::bindgen_prelude::Function<'env, (String, String), Option>, ) -> napi::Result> { + reject_on_output_reentry(&self.state)?; let raw_env = on_output.value().env; let tsfn = create_output_tsfn(on_output)?; let state = self.state.clone(); let promise = napi::bindgen_prelude::execute_tokio_future( raw_env, async move { + reject_on_output_reentry(&state)?; let mut bash = state.inner.lock().await; let cancelled = bash.cancellation_token(); let callback_requested_cancel = Arc::new(AtomicBool::new(false)); @@ -875,6 +917,7 @@ impl Bash { tsfn, cancelled.clone(), callback_requested_cancel.clone(), + state.on_output_reentry_depth.clone(), ); execute_rust_bash( &mut bash, @@ -1183,10 +1226,11 @@ impl Bash { /// Get a `JsFileSystem` handle for direct VFS operations. #[napi] - pub fn fs(&self) -> JsFileSystem { - JsFileSystem { + pub fn fs(&self) -> napi::Result { + reject_on_output_reentry(&self.state)?; + Ok(JsFileSystem { state: self.state.clone(), - } + }) } } @@ -1256,6 +1300,7 @@ impl BashTool { cancelled.clone(), callback_requested_cancel.clone(), callback_error.clone(), + s.on_output_reentry_depth.clone(), ); execute_rust_bash( &mut bash, @@ -1275,6 +1320,7 @@ impl BashTool { /// Execute bash commands asynchronously, returning a Promise. #[napi] pub async fn execute(&self, commands: String) -> napi::Result { + reject_on_output_reentry(&self.state)?; let s = self.state.clone(); let mut bash = s.inner.lock().await; execute_rust_bash(&mut bash, &commands, None, None, None, None).await @@ -1289,12 +1335,14 @@ impl BashTool { commands: String, on_output: napi::bindgen_prelude::Function<'env, (String, String), Option>, ) -> napi::Result> { + reject_on_output_reentry(&self.state)?; let raw_env = on_output.value().env; let tsfn = create_output_tsfn(on_output)?; let state = self.state.clone(); let promise = napi::bindgen_prelude::execute_tokio_future( raw_env, async move { + reject_on_output_reentry(&state)?; let mut bash = state.inner.lock().await; let cancelled = bash.cancellation_token(); let callback_requested_cancel = Arc::new(AtomicBool::new(false)); @@ -1302,6 +1350,7 @@ impl BashTool { tsfn, cancelled.clone(), callback_requested_cancel.clone(), + state.on_output_reentry_depth.clone(), ); execute_rust_bash( &mut bash, @@ -1654,10 +1703,11 @@ impl BashTool { /// Get a `JsFileSystem` handle for direct VFS operations. #[napi] - pub fn fs(&self) -> JsFileSystem { - JsFileSystem { + pub fn fs(&self) -> napi::Result { + reject_on_output_reentry(&self.state)?; + Ok(JsFileSystem { state: self.state.clone(), - } + }) } } @@ -2072,6 +2122,7 @@ fn shared_state_from_opts( .map_err(|e| napi::Error::from_reason(format!("Failed to create runtime: {e}")))?, ), cancelled: Arc::new(AtomicBool::new(false)), + on_output_reentry_depth: Arc::new(AtomicUsize::new(0)), username: opts.username.clone(), hostname: opts.hostname.clone(), max_commands: opts.max_commands, @@ -2105,6 +2156,7 @@ fn shared_state_from_opts( inner: Mutex::new(bash), rt: Mutex::new(rt), cancelled, + on_output_reentry_depth: tmp.on_output_reentry_depth, username: opts.username, hostname: opts.hostname, max_commands: opts.max_commands, diff --git a/crates/bashkit-js/wrapper.ts b/crates/bashkit-js/wrapper.ts index fcee7437..169bc2d8 100644 --- a/crates/bashkit-js/wrapper.ts +++ b/crates/bashkit-js/wrapper.ts @@ -126,8 +126,8 @@ export interface ExecuteOptions { * * Limitation: do not call back into the same `Bash` / `BashTool` instance * from this handler (`execute*`, `readFile`, `fs()`, etc.). The current - * binding delivers chunks while that instance is still mid-execution, so - * same-instance re-entry can deadlock or panic. + * binding rejects same-instance re-entry to avoid deadlocks and runtime + * panics. */ onOutput?: OnOutput; }