diff --git a/crates/tools/src/fs/contract_tests.rs b/crates/tools/src/fs/contract_tests.rs index 72ef60ceb0..16d0dff130 100644 --- a/crates/tools/src/fs/contract_tests.rs +++ b/crates/tools/src/fs/contract_tests.rs @@ -889,6 +889,232 @@ async fn sandbox_write_via_registry_sends_base64_to_bridge() { assert!(cmd.contains(&BASE64.encode(b"sandboxed write"))); } +#[tokio::test] +async fn sandbox_write_home_path_via_registry_uses_bridge_not_host_parent_resolution() { + use { + crate::{ + exec::ExecResult, + fs::sandbox_bridge::test_helpers::MockSandbox, + sandbox::{Sandbox, SandboxConfig, SandboxRouter}, + }, + base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}, + std::sync::Arc, + }; + + let mock = MockSandbox::new(vec![ExecResult { + stdout: String::new(), + stderr: String::new(), + exit_code: 0, + }]); + let backend: Arc = mock.clone(); + let router = Arc::new(SandboxRouter::with_backend( + SandboxConfig::default(), + backend, + )); + router.set_override("sandboxed", true).await; + + let mut registry = ToolRegistry::new(); + register_fs_tools(&mut registry, FsToolsContext { + sandbox_router: Some(router), + ..FsToolsContext::default() + }); + + let write = registry.get("Write").unwrap(); + let value = write + .execute(json!({ + "file_path": "/home/sandbox/write_probe.txt", + "content": "sandbox home write", + "_session_key": "sandboxed", + })) + .await + .unwrap(); + + assert_eq!(value["bytes_written"], 18); + let cmd = mock.last_command().unwrap(); + assert!(cmd.contains("/home/sandbox/write_probe.txt")); + assert!(cmd.contains(&BASE64.encode(b"sandbox home write"))); +} + +#[tokio::test] +async fn sandbox_read_and_write_workspace_paths_via_registry_use_bridge() { + use { + crate::{ + exec::ExecResult, + fs::sandbox_bridge::test_helpers::MockSandbox, + sandbox::{Sandbox, SandboxConfig, SandboxRouter}, + }, + base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}, + std::sync::Arc, + }; + + let workspace_file = moltis_config::data_dir().join("notes/workspace_probe.txt"); + let workspace_file = workspace_file.display().to_string(); + let mock = MockSandbox::new(vec![ + ExecResult { + stdout: BASE64.encode(b"workspace read"), + stderr: String::new(), + exit_code: 0, + }, + ExecResult { + stdout: String::new(), + stderr: String::new(), + exit_code: 0, + }, + ]); + let backend: Arc = mock.clone(); + let router = Arc::new(SandboxRouter::with_backend( + SandboxConfig::default(), + backend, + )); + router.set_override("sandboxed", true).await; + + let mut registry = ToolRegistry::new(); + register_fs_tools(&mut registry, FsToolsContext { + sandbox_router: Some(router), + ..FsToolsContext::default() + }); + + let read = registry.get("Read").unwrap(); + let value = read + .execute(json!({ + "file_path": workspace_file, + "_session_key": "sandboxed", + })) + .await + .unwrap(); + assert!( + value["content"] + .as_str() + .unwrap() + .contains("workspace read") + ); + + let write = registry.get("Write").unwrap(); + let value = write + .execute(json!({ + "file_path": workspace_file, + "content": "workspace write", + "_session_key": "sandboxed", + })) + .await + .unwrap(); + + assert_eq!(value["bytes_written"], 15); + let cmd = mock.last_command().unwrap(); + assert!(cmd.contains("notes/workspace_probe.txt")); + assert!(cmd.contains(&BASE64.encode(b"workspace write"))); +} + +#[tokio::test] +async fn sandbox_edit_home_path_via_registry_reads_and_writes_through_bridge() { + use { + crate::{ + exec::ExecResult, + fs::sandbox_bridge::test_helpers::MockSandbox, + sandbox::{Sandbox, SandboxConfig, SandboxRouter}, + }, + base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}, + std::sync::Arc, + }; + + let mock = MockSandbox::new(vec![ + ExecResult { + stdout: BASE64.encode(b"before\n"), + stderr: String::new(), + exit_code: 0, + }, + ExecResult { + stdout: String::new(), + stderr: String::new(), + exit_code: 0, + }, + ]); + let backend: Arc = mock.clone(); + let router = Arc::new(SandboxRouter::with_backend( + SandboxConfig::default(), + backend, + )); + router.set_override("sandboxed", true).await; + + let mut registry = ToolRegistry::new(); + register_fs_tools(&mut registry, FsToolsContext { + sandbox_router: Some(router), + ..FsToolsContext::default() + }); + + let edit = registry.get("Edit").unwrap(); + let value = edit + .execute(json!({ + "file_path": "/home/sandbox/edit_probe.txt", + "old_string": "before", + "new_string": "after", + "_session_key": "sandboxed", + })) + .await + .unwrap(); + + assert_eq!(value["replacements"], 1); + let cmd = mock.last_command().unwrap(); + assert!(cmd.contains("/home/sandbox/edit_probe.txt")); + assert!(cmd.contains(&BASE64.encode(b"after\n"))); +} + +#[tokio::test] +async fn sandbox_multi_edit_home_path_via_registry_reads_and_writes_through_bridge() { + use { + crate::{ + exec::ExecResult, + fs::sandbox_bridge::test_helpers::MockSandbox, + sandbox::{Sandbox, SandboxConfig, SandboxRouter}, + }, + base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}, + std::sync::Arc, + }; + + let mock = MockSandbox::new(vec![ + ExecResult { + stdout: BASE64.encode(b"alpha beta\n"), + stderr: String::new(), + exit_code: 0, + }, + ExecResult { + stdout: String::new(), + stderr: String::new(), + exit_code: 0, + }, + ]); + let backend: Arc = mock.clone(); + let router = Arc::new(SandboxRouter::with_backend( + SandboxConfig::default(), + backend, + )); + router.set_override("sandboxed", true).await; + + let mut registry = ToolRegistry::new(); + register_fs_tools(&mut registry, FsToolsContext { + sandbox_router: Some(router), + ..FsToolsContext::default() + }); + + let multi_edit = registry.get("MultiEdit").unwrap(); + let value = multi_edit + .execute(json!({ + "file_path": "/home/sandbox/multi_probe.txt", + "edits": [ + { "old_string": "alpha", "new_string": "ALPHA" }, + { "old_string": "beta", "new_string": "BETA" } + ], + "_session_key": "sandboxed", + })) + .await + .unwrap(); + + assert_eq!(value["edits_applied"], 2); + let cmd = mock.last_command().unwrap(); + assert!(cmd.contains("/home/sandbox/multi_probe.txt")); + assert!(cmd.contains(&BASE64.encode(b"ALPHA BETA\n"))); +} + #[tokio::test] async fn sandbox_write_serializes_same_file_mutations_via_registry() { let probe = ConcurrentExecProbeSandbox::new(); diff --git a/crates/tools/src/fs/edit.rs b/crates/tools/src/fs/edit.rs index 5be460b07c..8710737d84 100644 --- a/crates/tools/src/fs/edit.rs +++ b/crates/tools/src/fs/edit.rs @@ -476,7 +476,9 @@ impl AgentTool for EditTool { `old_string` is not unique in the file unless `replace_all=true`. \ This uniqueness requirement prevents the most common class of \ edit mistakes. Supply enough context in `old_string` to make the \ - match unique." + match unique. In sandboxed sessions, use absolute workspace/data paths \ + or `/home/sandbox/...` paths; both are read and written through the \ + sandbox when needed." } fn parameters_schema(&self) -> Value { diff --git a/crates/tools/src/fs/multi_edit.rs b/crates/tools/src/fs/multi_edit.rs index 5191902251..f16711b11f 100644 --- a/crates/tools/src/fs/multi_edit.rs +++ b/crates/tools/src/fs/multi_edit.rs @@ -316,7 +316,9 @@ impl AgentTool for MultiEditTool { "Apply multiple sequential edits to a single file as one atomic \ operation. Each edit sees the output of previous edits. Either all \ edits succeed or none are applied (full rollback on any failure). \ - Each edit follows the same uniqueness rules as `Edit`." + Each edit follows the same uniqueness rules as `Edit`. In sandboxed \ + sessions, use absolute workspace/data paths or `/home/sandbox/...` \ + paths; both are read and written through the sandbox when needed." } fn parameters_schema(&self) -> Value { diff --git a/crates/tools/src/fs/read/tool.rs b/crates/tools/src/fs/read/tool.rs index 0c52c1ce13..c2765484db 100644 --- a/crates/tools/src/fs/read/tool.rs +++ b/crates/tools/src/fs/read/tool.rs @@ -528,7 +528,9 @@ impl AgentTool for ReadTool { Supports `offset` (1-indexed line to start at) and `limit` (max lines \ to return) for paginating large files. Returns structured JSON with \ the file's content, total line count, and truncation flag. Binary \ - files return a typed marker instead of garbage." + files return a typed marker instead of garbage. In sandboxed sessions, \ + use absolute workspace/data paths or `/home/sandbox/...` paths; both \ + are routed through the sandbox when needed." } fn parameters_schema(&self) -> Value { diff --git a/crates/tools/src/fs/write.rs b/crates/tools/src/fs/write.rs index 81cc887a7b..75b70f6886 100644 --- a/crates/tools/src/fs/write.rs +++ b/crates/tools/src/fs/write.rs @@ -248,7 +248,9 @@ impl AgentTool for WriteTool { fn description(&self) -> &str { "Atomically write a file to the local filesystem. Parent directories \ must already exist. Refuses to follow symlinks. The entire file is \ - replaced — use `Edit` for surgical changes." + replaced — use `Edit` for surgical changes. In sandboxed sessions, \ + use absolute workspace/data paths or `/home/sandbox/...` paths; parent \ + directories must already exist in the target namespace." } fn parameters_schema(&self) -> Value { diff --git a/crates/tools/src/sandbox/docker.rs b/crates/tools/src/sandbox/docker.rs index 0ab20bfa27..e068642ff6 100644 --- a/crates/tools/src/sandbox/docker.rs +++ b/crates/tools/src/sandbox/docker.rs @@ -88,6 +88,18 @@ impl DockerSandbox { } } + #[cfg(test)] + pub(crate) fn with_cli(config: SandboxConfig, cli: &'static str) -> Self { + Self { + config, + kind: BackendKind::Docker, + cli, + backend_label: "docker", + provisioned: Mutex::new(HashSet::new()), + startup_gates: Mutex::new(HashMap::new()), + } + } + fn image(&self) -> &str { self.config .image @@ -758,7 +770,10 @@ impl Sandbox for DockerSandbox { max_bytes, ) .await?; - if !matches!(host_result, SandboxReadResult::NotFound) { + if matches!( + host_result, + SandboxReadResult::Ok(_) | SandboxReadResult::TooLarge(_) + ) { return Ok(host_result); } @@ -780,13 +795,24 @@ impl Sandbox for DockerSandbox { content: &[u8], ) -> Result> { if let Some(host_path) = self.mounted_host_path(id, file_path) { - return native_host_write_file( + let host_result = native_host_write_file( host_path .to_str() .ok_or_else(|| Error::message("mounted host path contains invalid UTF-8"))?, content, ) .await; + match host_result { + Ok(payload) => return Ok(payload), + Err(error) => { + debug!( + guest_path = file_path, + host_path = %host_path.display(), + %error, + "mounted host write failed; falling back to container write" + ); + }, + } } let container_name = self.container_name(id); @@ -795,13 +821,25 @@ impl Sandbox for DockerSandbox { async fn list_files(&self, id: &SandboxId, root: &str) -> Result { if let Some(host_path) = self.mounted_host_path(id, root) { - let host_files = native_host_list_files( + let host_result = native_host_list_files( host_path .to_str() .ok_or_else(|| Error::message("mounted host path contains invalid UTF-8"))?, ) - .await?; - return remap_host_list_result_to_guest(root, &host_path, host_files); + .await; + match host_result { + Ok(host_files) => { + return remap_host_list_result_to_guest(root, &host_path, host_files); + }, + Err(error) => { + debug!( + guest_path = root, + host_path = %host_path.display(), + %error, + "mounted host list failed; falling back to container list" + ); + }, + } } let container_name = self.container_name(id); diff --git a/crates/tools/src/sandbox/file_system.rs b/crates/tools/src/sandbox/file_system.rs index eabe953832..9733cdb51f 100644 --- a/crates/tools/src/sandbox/file_system.rs +++ b/crates/tools/src/sandbox/file_system.rs @@ -557,6 +557,17 @@ fn classify_container_copy_error(stderr: &str) -> Option None } +fn container_copy_failure_detail(stderr: &str, exit_code: Option) -> String { + let detail = stderr.trim(); + if !detail.is_empty() { + return detail.to_string(); + } + match exit_code { + Some(code) => format!("copy command exited with code {code} and no stderr"), + None => "copy command exited without a status code or stderr".to_string(), + } +} + async fn oci_exec_shell( cli: &str, container_name: &str, @@ -1084,7 +1095,7 @@ pub async fn oci_container_read_file( Err(Error::message(format!( "{cli} cp failed for '{file_path}': {}", - stderr.trim() + container_copy_failure_detail(stderr.trim(), status.code()) ))) }) .await @@ -1148,7 +1159,8 @@ pub async fn oci_container_write_file( } Err(Error::message(format!( - "{cli} cp failed for '{file_path}': {detail}" + "{cli} cp failed for '{file_path}': {}", + container_copy_failure_detail(detail, output.status.code()) ))) } @@ -1422,4 +1434,16 @@ mod tests { assert!(matches!(result, SandboxReadResult::NotFound)); } + + #[test] + fn oci_copy_failure_detail_is_not_empty_when_stderr_is_empty() { + assert_eq!( + container_copy_failure_detail("", Some(1)), + "copy command exited with code 1 and no stderr" + ); + assert_eq!( + container_copy_failure_detail("explicit failure", Some(1)), + "explicit failure" + ); + } } diff --git a/crates/tools/src/sandbox/tests/core.rs b/crates/tools/src/sandbox/tests/core.rs index 46f502667e..3b51619d59 100644 --- a/crates/tools/src/sandbox/tests/core.rs +++ b/crates/tools/src/sandbox/tests/core.rs @@ -610,6 +610,54 @@ async fn test_docker_write_file_uses_mounted_workspace_path() { ); } +#[cfg(unix)] +#[tokio::test] +async fn test_docker_write_file_falls_back_to_container_copy_when_host_mount_is_inaccessible() { + let temp_dir = tempfile::tempdir().unwrap(); + let fake_cli = temp_dir.path().join("fake-docker"); + std::fs::write( + &fake_cli, + "#!/bin/sh\n\ + if [ \"$1\" = \"exec\" ]; then printf 'missing-file\\n'; exit 0; fi\n\ + if [ \"$1\" = \"cp\" ]; then cat >/dev/null; exit 0; fi\n\ + exit 2\n", + ) + .unwrap(); + { + use std::os::unix::fs::PermissionsExt as _; + let mut permissions = std::fs::metadata(&fake_cli).unwrap().permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&fake_cli, permissions).unwrap(); + } + + let cli: &'static str = Box::leak(fake_cli.display().to_string().into_boxed_str()); + let host_data_dir = temp_dir.path().join("host-only-data"); + let docker = DockerSandbox::with_cli( + SandboxConfig { + workspace_mount: WorkspaceMount::Rw, + host_data_dir: Some(host_data_dir), + ..Default::default() + }, + cli, + ); + let id = SandboxId { + scope: SandboxScope::Session, + key: "test-docker-write-fallback".into(), + }; + let guest_file = moltis_config::data_dir().join("notes/todo.txt"); + + let result = docker + .write_file( + &id, + &guest_file.display().to_string(), + b"docker fallback write", + ) + .await + .unwrap(); + + assert!(result.is_none()); +} + #[tokio::test] async fn test_docker_write_file_uses_mounted_home_path() { let temp_dir = tempfile::tempdir().unwrap();