diff --git a/crates/native-sidecar/src/filesystem.rs b/crates/native-sidecar/src/filesystem.rs index cba1d96787..b007f72986 100644 --- a/crates/native-sidecar/src/filesystem.rs +++ b/crates/native-sidecar/src/filesystem.rs @@ -936,10 +936,47 @@ where path )) })?; - vm.kernel - .write_file(&path, bytes) - .map(|()| PythonVfsRpcResponsePayload::Empty) - .map_err(kernel_error) + (|| -> Result { + vm.kernel + .write_file(&path, bytes.clone()) + .map_err(kernel_error)?; + // Mounts own their writes; do not introduce an extra + // backend realpath/metadata requirement after success. + if is_non_root_mount_path(&vm.kernel, &path) { + return Ok(PythonVfsRpcResponsePayload::Empty); + } + // Python's VFS bridge writes complete file contents to the + // kernel. Keep the root staging copy current too, otherwise + // exit/read-side reconciliation imports API-seeded old bytes. + // Resolve through the trusted VFS after the permitted write: + // aliases must update their target, not replace a symlink, + // and non-root mounts must never leak into the root shadow. + let target = vm + .kernel + .filesystem() + .inner() + .realpath(&path) + .map_err(|error| kernel_error(error.into()))?; + if !is_non_root_mount_path(&vm.kernel, &target) { + // Guest-created files can remain kernel-only. Only + // refresh an existing staging copy, avoiding new host + // directory stubs that would overwrite guest metadata. + let shadow = shadow_host_path_for_guest(&vm.cwd, &target); + match fs::symlink_metadata(&shadow) { + Ok(_) => { + mirror_guest_file_write_to_shadow(&mut vm, &target, &bytes)?; + refresh_shadow_inventory_node(&mut vm, &target)?; + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(SidecarError::Io(format!( + "failed to inspect Python write shadow for {target}: {error}" + ))); + } + } + } + Ok(PythonVfsRpcResponsePayload::Empty) + })() } PythonVfsRpcMethod::Stat => vm .kernel @@ -1011,18 +1048,30 @@ where Ok(()) => { rename_guest_shadow_path(&mut vm, &path, &destination).and_then(|()| { forget_shadow_inventory_path(&mut vm, &path); - refresh_shadow_inventory_path(&mut vm, &destination)?; + // A kernel-only temporary file replacing an API- + // seeded destination removes its stale shadow. Do + // not inventory that absent shadow as a deletion + // to apply to the new kernel file on the next read. + if !is_non_root_mount_path(&vm.kernel, &destination) { + let shadow = shadow_host_path_for_guest(&vm.cwd, &destination); + match fs::symlink_metadata(&shadow) { + Ok(_) => refresh_shadow_inventory_path(&mut vm, &destination)?, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + forget_shadow_inventory_path(&mut vm, &destination); + } + Err(error) => return Err(SidecarError::Io(format!( + "failed to inspect Python rename shadow for {destination}: {error}" + ))), + } + } Ok(PythonVfsRpcResponsePayload::Empty) }) } Err(error) => Err(error), } } - // Kernel-direct (no shadow mirror): guest Python writes/creates - // land only in the kernel VFS, so mirroring create/modify ops into - // the host-side shadow would leave empty stubs that a later - // shadow->kernel sync resurrects over real content. (Delete/rename - // still mirror — to *remove* stale wire-written shadow entries.) + // Guest-created entries remain kernel-only. Write refreshes + // existing shadows with actual bytes, never empty file stubs. PythonVfsRpcMethod::Symlink => { let target = request.target.clone().ok_or_else(|| { SidecarError::InvalidState(format!( @@ -3539,7 +3588,7 @@ fn materialize_process_shadow_symlink( return Err(SidecarError::Io(format!( "failed to inspect process shadow symlink {}: {error}", shadow_path.display() - ))) + ))); } }; if !metadata.file_type().is_symlink() { @@ -5084,7 +5133,14 @@ fn mirror_guest_file_write_to_shadow( )) })?; - let stat = vm.kernel.lstat(&guest_path).map_err(kernel_error)?; + // Trusted bookkeeping after a permitted write must not require an extra + // guest fs.stat permission just to preserve the mirrored file's mode. + let stat = vm + .kernel + .filesystem() + .inner() + .lstat(&guest_path) + .map_err(|error| kernel_error(error.into()))?; fs::set_permissions(&shadow_path, fs::Permissions::from_mode(stat.mode & 0o7777)).map_err( |error| { SidecarError::Io(format!( diff --git a/crates/native-sidecar/tests/filesystem.rs b/crates/native-sidecar/tests/filesystem.rs index 4e1d2b8906..7e279d4d4d 100644 --- a/crates/native-sidecar/tests/filesystem.rs +++ b/crates/native-sidecar/tests/filesystem.rs @@ -1902,6 +1902,280 @@ try { guest_filesystem_call(sidecar, connection_id, session_id, vm_id, request_id, write); } + fn execute_python_code( + sidecar: &mut agentos_native_sidecar::NativeSidecar, + connection_id: &str, + session_id: &str, + vm_id: &str, + request_id: i64, + process_id: &str, + code: String, + ) -> String { + let response = sidecar + .dispatch_wire_blocking(wire_request( + request_id, + wire_vm(connection_id, session_id, vm_id), + RequestPayload::ExecuteRequest(ExecuteRequest { + process_id: process_id.to_owned(), + command: None, + runtime: Some(GuestRuntimeKind::Python), + entrypoint: Some(code), + args: Vec::new(), + env: HashMap::new(), + cwd: Some(String::from("/workspace")), + wasm_permission_tier: None, + retain_output: false, + }), + )) + .expect("execute Python code"); + assert!( + matches!( + response.response.payload, + ResponsePayload::ProcessStartedResponse(_) + ), + "Python did not start: {:?}", + response.response.payload + ); + let (stdout, stderr, exit) = + drain_process_output(sidecar, connection_id, session_id, vm_id, process_id); + assert_eq!(exit, Some(0), "{process_id}: {stderr}"); + stdout + } + + /// Python's VFS bridge must mirror the bytes it writes, not leave an old + /// API-created shadow to overwrite them at process exit or on a host read. + #[test] + fn python_edits_to_host_written_files_survive_shadow_reconciliation() { + support::assert_node_available(); + let mut sidecar = create_test_sidecar(); + let (connection_id, session_id) = authenticate_and_open_session(&mut sidecar); + let cwd = temp_dir("python-api-seeded-cwd"); + let (vm_id, _) = create_vm_wire( + &mut sidecar, + 3, + &connection_id, + &session_id, + GuestRuntimeKind::Python, + &cwd, + ); + let cases = [ + ("overwrite", "p.write_text('EDITED\\n')", "EDITED\n"), + ( + "read-modify-write", + "p.write_text(p.read_text() + 'added\\n')", + "original\nadded\n", + ), + ( + "replace", + "q = p.with_suffix('.tmp'); q.write_text('replaced\\n'); q.replace(p)", + "replaced\n", + ), + ]; + for (index, (name, mutation, expected)) in cases.into_iter().enumerate() { + let id = 20 + index as i64 * 10; + let path = format!("/workspace/{name}.txt"); + host_write_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + id, + &path, + "original\n", + ); + let code = format!( + "from pathlib import Path\np = Path({path:?})\n{mutation}\nprint(p.read_text(), end='')" + ); + let stdout = execute_python_code( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + id + 1, + &format!("python-{name}"), + code, + ); + assert_eq!(stdout, expected, "{name}: Python saw incorrect content"); + for read in 0..2 { + assert_eq!( + guest_read_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + id + 2 + read, + &path + ), + expected, + "{name}: host read lost the Python edit" + ); + } + assert_eq!( + execute_python_code( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + id + 4, + &format!("read-{name}"), + format!("from pathlib import Path\nprint(Path({path:?}).read_text(), end='')") + ), + expected, + "{name}: later Python execution lost the edit" + ); + } + dispose_vm_and_close_session(&mut sidecar, &connection_id, &session_id, &vm_id); + } + + #[test] + fn python_shadow_writes_preserve_symlinks_and_mount_boundaries() { + support::assert_node_available(); + let mut sidecar = create_test_sidecar(); + let (connection_id, session_id) = authenticate_and_open_session(&mut sidecar); + let cwd = temp_dir("python-shadow-mount-cwd"); + let (vm_id, _) = create_vm_wire( + &mut sidecar, + 3, + &connection_id, + &session_id, + GuestRuntimeKind::Python, + &cwd, + ); + let marker = format!( + "/workspace/marker-{}", + cwd.file_name().unwrap().to_string_lossy() + ); + host_write_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 5, + &marker, + "marker", + ); + let shadow = locate_shadow_root(&marker); + host_write_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 6, + "/workspace/target.txt", + "before", + ); + host_write_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 7, + "/mounted/data.txt", + "hidden root", + ); + let mount_dir = temp_dir("python-shadow-mount-data"); + fs::write(mount_dir.join("data.txt"), "mounted before").expect("seed mount"); + configure_vm_mounts( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + vec![MountDescriptor { + guest_path: String::from("/mounted"), + guest_source: String::from("host_dir"), + guest_fstype: String::from("host_dir"), + read_only: false, + plugin: MountPluginDescriptor { + id: String::from("host_dir"), + config: json!({"hostPath": mount_dir, "readOnly": false}).to_string(), + }, + }], + ); + let mut link = base_guest_filesystem_request( + GuestFilesystemOperation::Symlink, + "/workspace/alias.txt", + ); + link.target = Some(String::from("/workspace/target.txt")); + guest_filesystem_call(&mut sidecar, &connection_id, &session_id, &vm_id, 10, link); + execute_python_code( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 12, + "python-shadow-boundaries", + String::from( + "from pathlib import Path\nPath('/workspace/alias.txt').write_text('root after')\nPath('/mounted/data.txt').write_text('mounted after')\np = Path('/workspace/new/nested.txt')\np.parent.mkdir(mode=0o700)\np.write_text('new guest file')\n", + ), + ); + assert_eq!( + guest_read_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 13, + "/workspace/target.txt" + ), + "root after" + ); + assert_eq!( + guest_read_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 14, + "/workspace/new/nested.txt" + ), + "new guest file" + ); + assert!( + guest_lstat( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 15, + "/workspace/alias.txt" + ) + .is_symbolic_link + ); + assert_eq!( + fs::read_to_string(mount_dir.join("data.txt")).unwrap(), + "mounted after" + ); + assert!( + !shadow.join("workspace/new").exists(), + "guest-only directories must not acquire staging stubs" + ); + assert_eq!( + fs::read_to_string(shadow.join("mounted/data.txt")).unwrap(), + "hidden root", + "mounted bytes must not leak into the root shadow" + ); + configure_vm_mounts( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + Vec::new(), + ); + assert_eq!( + guest_read_text( + &mut sidecar, + &connection_id, + &session_id, + &vm_id, + 17, + "/mounted/data.txt" + ), + "hidden root" + ); + dispose_vm_and_close_session(&mut sidecar, &connection_id, &session_id, &vm_id); + fs::remove_dir_all(mount_dir).expect("remove test mount"); + } + /// Regression for #1961: a guest process that overwrites, appends to, or /// truncates a file the host seeded with `writeFile` must not have its /// edit replaced by the host-seeded bytes when the host (or a later