diff --git a/crates/bashkit-js/__test__/runtime-extension.spec.ts b/crates/bashkit-js/__test__/runtime-extension.spec.ts index 7aa63ad31..dd1eacee1 100644 --- a/crates/bashkit-js/__test__/runtime-extension.spec.ts +++ b/crates/bashkit-js/__test__/runtime-extension.spec.ts @@ -124,6 +124,18 @@ test("Bash: unmount retracts the replay, so reset does not resurrect it", (t) => t.not(bash.executeSync("cat /skills/my-skill/SKILL.md 2>&1").exitCode, 0); }); +test("Bash: unmount retracts an equivalent normalized mount path", (t) => { + const data = new FileSystem(); + data.writeFile("/SKILL.md", "# my-skill\n"); + + const bash = new Bash(); + bash.mount("/skills/staging/../my-skill", data); + bash.unmount("/skills/my-skill"); + bash.reset(); + + t.not(bash.executeSync("cat /skills/my-skill/SKILL.md 2>&1").exitCode, 0); +}); + test("Bash: reset preserves a runtime host directory mount", (t) => { const dir = mkdtempSync(path.join(tmpdir(), "bashkit-ext-")); try { diff --git a/crates/bashkit-js/src/lib.rs b/crates/bashkit-js/src/lib.rs index 2e5bef497..4500c3c91 100644 --- a/crates/bashkit-js/src/lib.rs +++ b/crates/bashkit-js/src/lib.rs @@ -1639,15 +1639,33 @@ fn record_runtime_env(log: &RuntimeEnvLog, key: &str, value: &str) { /// A mount at an already-recorded path replaces that record: the live VFS keeps /// one filesystem per mount point, so the replay must too. fn record_runtime_mount(log: &RuntimeMountLog, mount: RuntimeMount) { + let normalized = bashkit::normalize_path(Path::new(mount.vfs_path())) + .to_string_lossy() + .into_owned(); let mut mounts = log.lock().expect("runtime mount log poisoned"); - mounts.retain(|existing| existing.vfs_path() != mount.vfs_path()); - mounts.push(mount); + mounts.retain(|existing| existing.vfs_path() != normalized); + mounts.push(match mount { + RuntimeMount::Real { + host_path, + writable, + .. + } => RuntimeMount::Real { + host_path, + vfs_path: normalized, + writable, + }, + RuntimeMount::Fs { fs, .. } => RuntimeMount::Fs { + vfs_path: normalized, + fs, + }, + }); } /// Retract a recorded mount so `reset()` does not resurrect it after `unmount()`. fn forget_runtime_mount(log: &RuntimeMountLog, vfs_path: &str) { + let normalized = bashkit::normalize_path(Path::new(vfs_path)); let mut mounts = log.lock().expect("runtime mount log poisoned"); - mounts.retain(|existing| existing.vfs_path() != vfs_path); + mounts.retain(|existing| Path::new(existing.vfs_path()) != normalized); } /// Wrapper for the external handler that can be stored and cloned. diff --git a/crates/bashkit-python/src/lib.rs b/crates/bashkit-python/src/lib.rs index 6f7736740..0e24a7392 100644 --- a/crates/bashkit-python/src/lib.rs +++ b/crates/bashkit-python/src/lib.rs @@ -4106,20 +4106,24 @@ fn record_runtime_mount( vfs_path: &str, fs: Arc, ) -> PyResult<()> { + let vfs_path = bashkit::normalize_path(Path::new(vfs_path)) + .to_string_lossy() + .into_owned(); let mut mounts = log .lock() .map_err(|_| PyRuntimeError::new_err("runtime mount log poisoned"))?; - mounts.retain(|(path, _)| path != vfs_path); - mounts.push((vfs_path.to_string(), fs)); + mounts.retain(|(path, _)| path != &vfs_path); + mounts.push((vfs_path, fs)); Ok(()) } /// Retract a recorded mount so `reset()` does not resurrect it after `unmount()`. fn forget_runtime_mount(log: &RuntimeMountLog, vfs_path: &str) -> PyResult<()> { + let vfs_path = bashkit::normalize_path(Path::new(vfs_path)); let mut mounts = log .lock() .map_err(|_| PyRuntimeError::new_err("runtime mount log poisoned"))?; - mounts.retain(|(path, _)| path != vfs_path); + mounts.retain(|(path, _)| Path::new(path) != vfs_path); Ok(()) } diff --git a/crates/bashkit-python/tests/test_runtime_extension.py b/crates/bashkit-python/tests/test_runtime_extension.py index 3371a12d9..255a314f5 100644 --- a/crates/bashkit-python/tests/test_runtime_extension.py +++ b/crates/bashkit-python/tests/test_runtime_extension.py @@ -115,6 +115,14 @@ def test_unmount_retracts_replay(): assert bash.execute_sync("cat /skills/my-skill/SKILL.md 2>&1").exit_code != 0 +def test_unmount_retracts_equivalent_normalized_mount_path(): + bash = Bash() + bash.mount("/skills/staging/../my-skill", skill_filesystem()) + bash.unmount("/skills/my-skill") + bash.reset() + assert bash.execute_sync("cat /skills/my-skill/SKILL.md 2>&1").exit_code != 0 + + def test_tool_reset_preserves_runtime_mount(): tool = BashTool() tool.mount("/skills/my-skill", skill_filesystem()) diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 673f89e9b..66f9c5983 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -872,7 +872,7 @@ Only exact domain matches are allowed (TM-NET-017). | TM-ISO-022 | `$?` leaks across `exec()` calls | Exit code from one `exec()` visible as `$?` in next `exec()` instead of resetting to 0 | `reset_transient_state()` zeroes `last_exit_code` at the start of every `exec()` | **MITIGATED** | | TM-ISO-023 | `set -e` leaks across `exec()` calls | `set` options (`-e`, `-x`, etc.) persist across `exec()` calls, causing unexpected abort behavior | `reset_transient_state()` clears `SET_OPTION_VARS` | **FIXED** | | TM-ISO-024 | `$?` leaks into VFS subprocess | Parent `last_exit_code` visible inside VFS script subprocess, causing false `set -e` failures | `execute_script_content()` sets `last_exit_code = 0`, clears `nounset_error`, and clears `traps` for the child | **MITIGATED** | -| TM-ISO-025 | Wrapper rebuild silently drops constructor capabilities | A binding's `reset()` or fresh-execution path rebuilds `Bash` without limits, policy files, callbacks, network policy, or other host configuration, changing the sandbox contract after the first call | The canonical capability manifest requires executable evidence for every supported surface cell. NAPI `SharedState` retains constructor `files`, and `build_bash_from_state` is the single rebuild path used by construction and reset. Host mutations applied *after* construction are recorded on the same path: NAPI `runtime_env`/`runtime_mounts` and the Python equivalents replay `set_env()` and runtime mounts on every rebuild, and `unmount()` retracts its record so a rebuild cannot resurrect a removed mount. Script-set env is not recorded, so `reset()` still discards it. Regressions: `Bash: reset restores configured files`, `Bash: reset preserves setEnv values`, `Bash: reset preserves a runtime FileSystem mount`, `Bash: unmount retracts the replay, so reset does not resurrect it`, `test_reset_preserves_set_env`, `test_unmount_retracts_replay` | **MITIGATED** | +| TM-ISO-025 | Wrapper rebuild silently drops or restores revoked capabilities | A binding's `reset()` or fresh-execution path rebuilds `Bash` without limits, policy files, callbacks, network policy, or other host configuration, or replays an explicitly revoked mount, changing the sandbox contract after the first call | The canonical capability manifest requires executable evidence for every supported surface cell. NAPI `SharedState` retains constructor `files`, and `build_bash_from_state` is the single rebuild path used by construction and reset. Host mutations applied *after* construction are recorded on the same path: NAPI `runtime_env`/`runtime_mounts` and the Python equivalents replay `set_env()` and runtime mounts on every rebuild. Runtime mount replay keys use the VFS-normalized mount point, so `unmount()` retracts the record even when mount and unmount callers use equivalent paths containing `.` or `..`; a rebuild cannot resurrect the removed mount. Script-set env is not recorded, so `reset()` still discards it. Regressions: `Bash: reset restores configured files`, `Bash: reset preserves setEnv values`, `Bash: reset preserves a runtime FileSystem mount`, `Bash: unmount retracts the replay, so reset does not resurrect it`, `Bash: unmount retracts an equivalent normalized mount path`, `test_reset_preserves_set_env`, `test_unmount_retracts_replay`, `test_unmount_retracts_equivalent_normalized_mount_path` | **MITIGATED** | | TM-ISO-026 | Shared ToolRegistry leaks tenant identity or traces across requests/runtimes | One immutable registry and callback set may serve concurrent tenants through shell, Python, and TypeScript; registry-global mutable request state would cross-contaminate authorization and telemetry | `ToolCallRequest` carries tenant + a fresh bounded trace through `ExecutionExtensions`; runtime bridges copy it into a Tokio task-local only for the current suspension; callbacks receive owned `ToolArgs` context; no tenant value is stored in the registry (`tool_registry.rs`) | **MITIGATED** | | TM-ISO-027 | Request-owned execution authority or retained facility handles survive completion | A retained runtime, transport, callback, budget, extension, or custom-builtin VFS handle emits late output, charges a reused request, or accesses facilities after completion/cancellation | One `ExecutionBudget` and one capability lease are created before initialization and closed/revoked on every exit. The budget gates work and async results; `ExecutionCapability`, scoped VFS wrappers, host-call brokers, and `ToolArgs` context gate retained facilities. Cleanup is bounded/idempotent and reports only sanitized counts. `BuiltinRegistry::insert_trusted` explicitly models the intentional unscoped host escape hatch. Shared lifecycle evidence is defined in [Request Execution Lifecycle](request-lifecycle.md); Rust and Python capability regressions cover late/cross-request/cancellation use and cleanup failure. | **MITIGATED** |