From c78f82301eb4145c4ffb35e5c5170ac31057f99e Mon Sep 17 00:00:00 2001 From: Sree Narayanan Date: Fri, 23 Oct 2026 00:07:34 +0400 Subject: [PATCH 1/2] fix(runtime): make the kernel own the guest process cwd process.chdir now goes through a process.chdir RPC that validates the target with the same mapping-aware stat as fs.stat and records it in the kernel process table, like umask. The sidecar resolves every relative JavaScript fs path against that cwd at its one path entry point, and a child spawn starts in, and resolves a relative cwd option against, the parent's kernel cwd. Relative fs calls no longer land in /, and /proc//cwd and child processes follow chdir. --- crates/bridge/bridge-contract.json | 1 + crates/kernel/src/kernel.rs | 21 ++ crates/kernel/src/process_table.rs | 16 + .../src/execution/child_process.rs | 42 +++ .../src/execution/javascript/rpc.rs | 30 +- crates/native-sidecar/src/filesystem.rs | 275 ++++++++++++++---- .../tests/builtin_conformance.rs | 67 +++++ .../bridge-src/builtins/process.ts | 39 ++- .../build-tools/bridge-src/global-exposure.ts | 5 + .../scripts/check-bridge-contract.mjs | 1 + 10 files changed, 408 insertions(+), 89 deletions(-) diff --git a/crates/bridge/bridge-contract.json b/crates/bridge/bridge-contract.json index 7ca79d2912..5a85af89b4 100644 --- a/crates/bridge/bridge-contract.json +++ b/crates/bridge/bridge-contract.json @@ -245,6 +245,7 @@ ], "returnType": "unknown", "names": [ + "process.chdir", "process.cpuUsage", "process.fcntlLock", "process.flock", diff --git a/crates/kernel/src/kernel.rs b/crates/kernel/src/kernel.rs index f9f4a05371..be8f192276 100644 --- a/crates/kernel/src/kernel.rs +++ b/crates/kernel/src/kernel.rs @@ -1673,6 +1673,27 @@ impl KernelVm { } } + /// The process working directory, which relative path lookup, child + /// spawns and `/proc//cwd` read. + pub fn cwd(&self, requester_driver: &str, pid: u32) -> KernelResult { + self.assert_driver_owns(requester_driver, pid)?; + Ok(self.processes.get_cwd(pid)?) + } + + /// Records a new working directory after the caller has resolved `cwd` to + /// an existing directory, as `chdir(2)` does. + pub fn set_cwd(&mut self, requester_driver: &str, pid: u32, cwd: &str) -> KernelResult<()> { + self.assert_not_terminated()?; + self.assert_driver_owns(requester_driver, pid)?; + if !cwd.starts_with('/') { + return Err(KernelError::new( + "EINVAL", + format!("process cwd must be absolute, got {cwd:?}"), + )); + } + Ok(self.processes.set_cwd(pid, normalize_path(cwd))?) + } + pub fn exists(&self, path: &str) -> KernelResult { self.assert_not_terminated()?; self.exists_internal(None, path) diff --git a/crates/kernel/src/process_table.rs b/crates/kernel/src/process_table.rs index b8e4513da4..fc86aaeb73 100644 --- a/crates/kernel/src/process_table.rs +++ b/crates/kernel/src/process_table.rs @@ -852,6 +852,22 @@ impl ProcessTable { .ok_or_else(|| ProcessTableError::no_such_process(pid)) } + pub fn get_cwd(&self, pid: u32) -> ProcessResult { + self.get(pid) + .map(|entry| entry.cwd) + .ok_or_else(|| ProcessTableError::no_such_process(pid)) + } + + pub fn set_cwd(&self, pid: u32, cwd: String) -> ProcessResult<()> { + let mut state = self.inner.lock_state(); + let record = state + .entries + .get_mut(&pid) + .ok_or_else(|| ProcessTableError::no_such_process(pid))?; + record.entry.cwd = cwd; + Ok(()) + } + pub fn get_umask(&self, pid: u32) -> ProcessResult { self.get(pid) .map(|entry| entry.umask) diff --git a/crates/native-sidecar/src/execution/child_process.rs b/crates/native-sidecar/src/execution/child_process.rs index 60e7e8b60e..b068610f49 100644 --- a/crates/native-sidecar/src/execution/child_process.rs +++ b/crates/native-sidecar/src/execution/child_process.rs @@ -2320,6 +2320,34 @@ pub(super) fn apply_posix_spawn_file_actions( )) } +/// A child starts in its parent's current directory and resolves a relative +/// `cwd` option against it, as `chdir` after `fork` does. The parent's +/// `guest_cwd` is its launch-time host mapping anchor, which `process.chdir` +/// does not move, so the current directory comes from the kernel. +fn start_child_in_parent_cwd( + vm: &VmState, + parent: &ActiveProcess, + request: &mut JavascriptChildProcessSpawnRequest, +) -> Result<(), SidecarError> { + let current = vm + .kernel + .cwd(EXECUTION_DRIVER_NAME, parent.kernel_pid) + .map_err(kernel_error)?; + match request.options.cwd.as_deref() { + Some(cwd) if !cwd.starts_with('/') => { + request.options.cwd = Some(normalize_path(&format!( + "{}/{cwd}", + current.trim_end_matches('/') + ))); + } + None if current != normalize_path(&parent.guest_cwd) => { + request.options.cwd = Some(current); + } + _ => {} + } + Ok(()) +} + pub(super) fn resolve_posix_spawn_action_path(cwd: &str, action_path: &str) -> String { if action_path.starts_with('/') { normalize_path(action_path) @@ -6029,6 +6057,20 @@ where let requested_pgid = spawn_attributes.process_group; let current_process_label = NativeSidecar::::child_process_path_label(process_id, current_process_path); + { + let vm = vms.get(vm_id).ok_or_else(|| missing_vm_error(vm_id))?; + let root = vm + .active_processes + .get(process_id) + .ok_or_else(|| missing_process_error(vm_id, process_id))?; + let parent = NativeSidecar::::active_process_by_path(root, current_process_path) + .ok_or_else(|| { + SidecarError::InvalidState(format!( + "unknown child process path {current_process_label} during spawn cwd resolution" + )) + })?; + start_child_in_parent_cwd(&vm, parent, &mut request)?; + } let parent_sync_roots = { let vm = vms.get(vm_id).ok_or_else(|| missing_vm_error(vm_id))?; let root = vm diff --git a/crates/native-sidecar/src/execution/javascript/rpc.rs b/crates/native-sidecar/src/execution/javascript/rpc.rs index 638ec113b4..916bd4b258 100644 --- a/crates/native-sidecar/src/execution/javascript/rpc.rs +++ b/crates/native-sidecar/src/execution/javascript/rpc.rs @@ -1,6 +1,7 @@ use super::super::*; use crate::filesystem::{ - javascript_sync_rpc_path_arg, remove_process_shadow_path, rename_process_shadow_path, + javascript_fs_stat_value, javascript_sync_rpc_path_arg, remove_process_shadow_path, + rename_process_shadow_path, }; use agentos_kernel::vfs::{VirtualTimeSpec, VirtualUtimeSpec}; @@ -641,8 +642,13 @@ where } if request.raw_bytes_args.contains_key(&usize::MAX) && request.method == "fs.readFileRangeSync" { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem ranged read path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem ranged read path", + )?; let offset = javascript_sync_rpc_arg_u64(&request.args, 1, "filesystem ranged read offset")?; let length = usize::try_from(javascript_sync_rpc_arg_u64( @@ -2056,6 +2062,22 @@ where "action": "default", })) } + "process.chdir" => { + let path = + javascript_sync_rpc_path_arg(kernel, process, &request.args, 0, "chdir path")?; + let stat = javascript_fs_stat_value(kernel, process, process.kernel_pid, &path)?; + if stat.get("isDirectory") != Some(&Value::Bool(true)) { + return Err(SidecarError::Errno { + errno: "ENOTDIR", + message: format!("not a directory: {path}"), + }); + } + kernel + .set_cwd(EXECUTION_DRIVER_NAME, process.kernel_pid, &path) + .and_then(|()| kernel.cwd(EXECUTION_DRIVER_NAME, process.kernel_pid)) + .map(Value::String) + .map_err(kernel_error) + } "process.umask" => { let new_mask = javascript_sync_rpc_arg_u32_optional(&request.args, 0, "process umask")?; kernel @@ -2342,7 +2364,7 @@ fn mirror_process_chmod_to_host( request: &JavascriptSyncRpcRequest, ) -> Result<(), SidecarError> { let guest_path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem chmod path")?; + javascript_sync_rpc_path_arg(kernel, process, &request.args, 0, "filesystem chmod path")?; let guest_path = if guest_path.starts_with('/') { normalize_path(&guest_path) } else { diff --git a/crates/native-sidecar/src/filesystem.rs b/crates/native-sidecar/src/filesystem.rs index 625b03a6aa..68bf0c256c 100644 --- a/crates/native-sidecar/src/filesystem.rs +++ b/crates/native-sidecar/src/filesystem.rs @@ -1576,8 +1576,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( match request.method.as_str() { "fs.open" | "fs.openSync" => { let phase_start = Instant::now(); - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem open path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem open path", + )?; let path = path.as_str(); let flags = javascript_sync_rpc_arg_u32(&request.args, 1, "filesystem open flags")?; let mode = @@ -1812,8 +1817,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.openTmpfileSync" => { - let directory = - javascript_sync_rpc_path_arg(process, &request.args, 0, "unnamed-file directory")?; + let directory = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "unnamed-file directory", + )?; let flags = javascript_sync_rpc_arg_u32(&request.args, 1, "unnamed-file open flags")?; let mode = javascript_sync_rpc_arg_u32(&request.args, 2, "unnamed-file mode")?; let linkable = @@ -1833,6 +1843,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( "fs.linkFdSync" => { let fd = javascript_sync_rpc_arg_u32(&request.args, 0, "unnamed-file fd")?; let destination = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 1, @@ -1898,6 +1909,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.truncateSync" | "fs.truncateForProcessSync" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2019,8 +2031,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .collect::>())) } "fs.chmodForProcessSync" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem chmod path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem chmod path", + )?; let mode = javascript_sync_rpc_arg_u32(&request.args, 1, "filesystem chmod mode")?; let mut result = kernel.chmod_for_process(EXECUTION_DRIVER_NAME, kernel_pid, &path, mode); @@ -2119,6 +2136,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.readFileSync" | "fs.promises.readFile" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2160,6 +2178,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.writeFileSync" | "fs.promises.writeFile" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2209,8 +2228,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( Ok(Value::Null) } "fs.statfsSync" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem statfs path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem statfs path", + )?; let stats = kernel .filesystem_stats_for_process(EXECUTION_DRIVER_NAME, kernel_pid, path.as_str()) .map_err(kernel_error)?; @@ -2223,34 +2247,23 @@ pub(crate) fn service_javascript_fs_sync_rpc( })) } "fs.statSync" | "fs.promises.stat" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem stat path")?; - let path = path.as_str(); - if let Some(mapped_host) = mapped_runtime_host_path_for_read(kernel, process, path) { - materialize_mapped_host_path_from_kernel(kernel, kernel_pid, path, &mapped_host)?; - let opened = open_mapped_runtime_beneath( - &mapped_host, - "fs.stat", - O_PATH_ANCHOR, - Mode::empty(), - )?; - let metadata = opened.handle.metadata().map_err(|error| { - SidecarError::Io(format!( - "failed to stat mapped guest path {} -> {}: {error}", - path, - opened.host_path.display() - )) - })?; - return Ok(metadata.to_value()); - } - kernel - .stat_for_process(EXECUTION_DRIVER_NAME, kernel_pid, path) - .map(javascript_sync_rpc_stat_value) - .map_err(kernel_error) + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem stat path", + )?; + javascript_fs_stat_value(kernel, process, kernel_pid, &path) } "fs.lstatSync" | "fs.promises.lstat" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem lstat path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem lstat path", + )?; let path = path.as_str(); if let Some(mapped_host) = mapped_runtime_host_path_for_read(kernel, process, path) { materialize_mapped_host_path_from_kernel(kernel, kernel_pid, path, &mapped_host)?; @@ -2263,15 +2276,25 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.readdirSync" | "fs.promises.readdir" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem readdir path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem readdir path", + )?; let path = path.as_str(); service_javascript_fs_readdir_entries(kernel, process, kernel_pid, path) .map(javascript_sync_rpc_readdir_typed_value) } "fs.mkdirSync" | "fs.promises.mkdir" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem mkdir path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem mkdir path", + )?; let path = path.as_str(); let recursive = javascript_sync_rpc_option_bool(&request.args, 1, "recursive").unwrap_or(false); @@ -2310,8 +2333,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.mknodSync" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem mknod path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem mknod path", + )?; let mode = javascript_sync_rpc_arg_u32(&request.args, 1, "filesystem mknod mode")?; let rdev = javascript_sync_rpc_arg_u64(&request.args, 2, "filesystem mknod device")?; kernel @@ -2320,8 +2348,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.remountSync" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem remount path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem remount path", + )?; let options = request.args.get(1).and_then(Value::as_str).ok_or_else(|| { SidecarError::InvalidState(String::from( "filesystem remount options must be a string", @@ -2338,8 +2371,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.accessSync" | "fs.promises.access" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem access path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem access path", + )?; let path = path.as_str(); let mode = javascript_sync_rpc_arg_u32_optional(&request.args, 1, "filesystem access mode")? @@ -2376,6 +2414,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.copyFileSync" | "fs.promises.copyFile" => { let source = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2383,6 +2422,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( )?; let source = source.as_str(); let destination = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 1, @@ -2480,8 +2520,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.existsSync" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem exists path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem exists path", + )?; let path = path.as_str(); if let Some(mapped_host) = mapped_runtime_host_path_for_read(kernel, process, path) { materialize_mapped_host_path_from_kernel(kernel, kernel_pid, path, &mapped_host)?; @@ -2503,6 +2548,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.readlinkSync" | "fs.promises.readlink" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2522,8 +2568,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( "fs.symlinkSync" | "fs.promises.symlink" => { let target = javascript_sync_rpc_arg_str(&request.args, 0, "filesystem symlink target")?; - let link_path = - javascript_sync_rpc_path_arg(process, &request.args, 1, "filesystem symlink path")?; + let link_path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 1, + "filesystem symlink path", + )?; let link_path = link_path.as_str(); match mapped_runtime_host_path(kernel, process, link_path, true) { Some(MappedRuntimeHostAccess::Writable(mapped_host)) => { @@ -2551,11 +2602,21 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.linkSync" | "fs.promises.link" => { - let source = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem link source")?; + let source = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem link source", + )?; let source = source.as_str(); - let destination = - javascript_sync_rpc_path_arg(process, &request.args, 1, "filesystem link path")?; + let destination = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 1, + "filesystem link path", + )?; let destination = destination.as_str(); kernel .link_for_process(EXECUTION_DRIVER_NAME, kernel_pid, source, destination) @@ -2564,6 +2625,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.renameSync" | "fs.promises.rename" => { let source = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2571,6 +2633,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( )?; let source = source.as_str(); let destination = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 1, @@ -2600,6 +2663,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.renameAt2Sync" => { let source = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2607,6 +2671,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( )?; let source = source.as_str(); let destination = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 1, @@ -2645,8 +2710,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( Ok(Value::Null) } "fs.rmdirSync" | "fs.promises.rmdir" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem rmdir path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem rmdir path", + )?; let path = path.as_str(); match mapped_runtime_host_path(kernel, process, path, true) { Some(MappedRuntimeHostAccess::Writable(mapped_host)) => { @@ -2685,8 +2755,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( Ok(Value::Null) } "fs.unlinkSync" | "fs.promises.unlink" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem unlink path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem unlink path", + )?; let path = path.as_str(); match mapped_runtime_host_path(kernel, process, path, true) { Some(MappedRuntimeHostAccess::Writable(mapped_host)) => { @@ -2743,8 +2818,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( Ok(Value::Null) } "fs.chmodSync" | "fs.promises.chmod" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem chmod path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem chmod path", + )?; let path = path.as_str(); let mode = javascript_sync_rpc_arg_u32(&request.args, 1, "filesystem chmod mode")?; match mapped_runtime_host_path(kernel, process, path, true) { @@ -2789,8 +2869,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.chownSync" | "fs.promises.chown" | "fs.lchownSync" | "fs.promises.lchown" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem chown path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem chown path", + )?; let path = path.as_str(); let uid = javascript_sync_rpc_arg_u32(&request.args, 1, "filesystem chown uid")?; let gid = javascript_sync_rpc_arg_u32(&request.args, 2, "filesystem chown gid")?; @@ -2814,6 +2899,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.getxattrSync" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2836,6 +2922,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.listxattrSync" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2856,6 +2943,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.setxattrSync" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2889,6 +2977,7 @@ pub(crate) fn service_javascript_fs_sync_rpc( } "fs.removexattrSync" => { let path = javascript_sync_rpc_path_arg( + kernel, process, &request.args, 0, @@ -2910,8 +2999,13 @@ pub(crate) fn service_javascript_fs_sync_rpc( .map_err(kernel_error) } "fs.utimesSync" | "fs.promises.utimes" | "fs.lutimesSync" | "fs.promises.lutimes" => { - let path = - javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem utimes path")?; + let path = javascript_sync_rpc_path_arg( + kernel, + process, + &request.args, + 0, + "filesystem utimes path", + )?; let path = path.as_str(); let atime = parse_utime_arg(&request.args, 1, "filesystem utimes atime")?; let mtime = parse_utime_arg(&request.args, 2, "filesystem utimes mtime")?; @@ -3082,14 +3176,43 @@ fn kernel_fd_surfaces_stdio_event( )) } +/// `stat(2)` for a resolved guest path, following symlinks, through the same +/// host-mapped or kernel route as `fs.statSync`. +pub(crate) fn javascript_fs_stat_value( + kernel: &mut SidecarKernel, + process: &ActiveProcess, + kernel_pid: u32, + path: &str, +) -> Result { + if let Some(mapped_host) = mapped_runtime_host_path_for_read(kernel, process, path) { + materialize_mapped_host_path_from_kernel(kernel, kernel_pid, path, &mapped_host)?; + let opened = + open_mapped_runtime_beneath(&mapped_host, "fs.stat", O_PATH_ANCHOR, Mode::empty())?; + let metadata = opened.handle.metadata().map_err(|error| { + SidecarError::Io(format!( + "failed to stat mapped guest path {} -> {}: {error}", + path, + opened.host_path.display() + )) + })?; + return Ok(metadata.to_value()); + } + kernel + .stat_for_process(EXECUTION_DRIVER_NAME, kernel_pid, path) + .map(javascript_sync_rpc_stat_value) + .map_err(kernel_error) +} + pub(crate) fn javascript_sync_rpc_path_arg( + kernel: &SidecarKernel, process: &ActiveProcess, args: &[Value], index: usize, label: &str, ) -> Result { let path = javascript_sync_rpc_arg_str(args, index, label)?; - let path = normalize_process_filesystem_rpc_path(process, path); + let path = resolve_javascript_process_path(kernel, process, path, label)?; + let path = normalize_process_filesystem_rpc_path(process, &path); if path.split('/').any(is_internal_unnamed_file_name) { return Err(SidecarError::Kernel(format!( "ENOENT: no such file or directory: {path}" @@ -3098,6 +3221,29 @@ pub(crate) fn javascript_sync_rpc_path_arg( Ok(path) } +/// Starts a relative guest path at the process cwd, as Linux path lookup does. +/// The cwd comes from the kernel because `process.chdir` moves it after spawn. +fn resolve_javascript_process_path( + kernel: &SidecarKernel, + process: &ActiveProcess, + path: &str, + label: &str, +) -> Result { + if path.is_empty() { + return Err(SidecarError::Errno { + errno: "ENOENT", + message: format!("no such file or directory: empty {label}"), + }); + } + if path.starts_with('/') { + return Ok(path.to_owned()); + } + let cwd = kernel + .cwd(EXECUTION_DRIVER_NAME, process.kernel_pid) + .map_err(kernel_error)?; + Ok(format!("{}/{}", cwd.trim_end_matches('/'), path)) +} + fn normalize_process_filesystem_rpc_path(process: &ActiveProcess, path: &str) -> String { let host_path = Path::new(path); if host_path.is_absolute() { @@ -5010,7 +5156,8 @@ pub(crate) fn service_javascript_fs_readdir_raw_sync_rpc( kernel_pid: u32, request: &JavascriptSyncRpcRequest, ) -> Result, SidecarError> { - let path = javascript_sync_rpc_path_arg(process, &request.args, 0, "filesystem readdir path")?; + let path = + javascript_sync_rpc_path_arg(kernel, process, &request.args, 0, "filesystem readdir path")?; let entries = service_javascript_fs_readdir_entries(kernel, process, kernel_pid, path.as_str())?; encode_javascript_readdir_raw_payload(entries) diff --git a/crates/native-sidecar/tests/builtin_conformance.rs b/crates/native-sidecar/tests/builtin_conformance.rs index de8e14c604..c3c49e46c6 100644 --- a/crates/native-sidecar/tests/builtin_conformance.rs +++ b/crates/native-sidecar/tests/builtin_conformance.rs @@ -2360,6 +2360,70 @@ fn rm_does_not_follow_symlinks_matches_host_node() { run_isolated_builtin_conformance_test("fs-rm-symlinks"); } +// The probe chdirs into a VM-local directory, so relative paths cannot be +// rescued by the host-mapped harness cwd. +fn process_chdir_moves_relative_paths_and_children_matches_host_node_impl() { + assert_conformance( + "process-chdir-cwd", + r#" +import fs from "node:fs"; +import path from "node:path"; +import { execFileSync } from "node:child_process"; + +const base = fs.realpathSync(fs.mkdtempSync("/tmp/agentos-cwd-")); +const rel = (target) => path.relative(base, target); +const childCwd = (options) => + rel(execFileSync("node", ["-e", "process.stdout.write(process.cwd())"], options).toString()); +fs.mkdirSync(path.join(base, "p/sub"), { recursive: true }); +process.chdir(path.join(base, "p")); + +fs.mkdirSync("made"); +fs.symlinkSync("made", "link"); +const relativeOps = { + entries: fs.readdirSync(process.cwd()).sort(), + linkIsSymlink: fs.lstatSync("link").isSymbolicLink(), + strayAtRoot: ["made", "link"].filter((name) => fs.existsSync(`/${name}`)), +}; +fs.unlinkSync("link"); +fs.rmdirSync("made"); + +process.chdir("sub"); +const cwd = { process: rel(process.cwd()), proc: rel(fs.readlinkSync("/proc/self/cwd")) }; +const child = { inherited: childCwd(), relativeOption: childCwd({ cwd: ".." }) }; + +const chdirError = (target) => { + try { + process.chdir(target); + return null; + } catch (error) { + return { + code: error.code, + syscall: error.syscall, + path: rel(error.path), + dest: error.dest, + message: error.message.replaceAll(base, ""), + }; + } +}; +fs.writeFileSync("file.txt", "x"); +const errors = { + missing: chdirError("missing"), + notDirectory: chdirError("file.txt"), + cwdAfterFailures: rel(process.cwd()), +}; + +process.chdir("/"); +fs.rmSync(base, { recursive: true }); +console.log(JSON.stringify({ relativeOps, cwd, child, errors })); +"#, + ); +} + +#[test] +fn process_chdir_moves_relative_paths_and_children_matches_host_node() { + run_isolated_builtin_conformance_test("process-chdir-cwd"); +} + fn console_conformance_matches_host_node() { assert_conformance( "console", @@ -5133,6 +5197,9 @@ fn __builtin_conformance_extra_test_runner() { "http-server-unref-exit" => unrefd_http_server_does_not_keep_guest_process_alive_impl(), "fs-write-file-numeric-fd" => write_file_sync_numeric_fd_matches_host_node_impl(), "fs-rm-symlinks" => rm_does_not_follow_symlinks_matches_host_node_impl(), + "process-chdir-cwd" => { + process_chdir_moves_relative_paths_and_children_matches_host_node_impl() + } other => panic!("unknown builtin conformance extra test: {other}"), } } diff --git a/packages/build-tools/bridge-src/builtins/process.ts b/packages/build-tools/bridge-src/builtins/process.ts index 4c7aac0be5..2e57185def 100644 --- a/packages/build-tools/bridge-src/builtins/process.ts +++ b/packages/build-tools/bridge-src/builtins/process.ts @@ -2,7 +2,7 @@ import { encodeChildProcessIpcFrame, splitChildProcessIpcFrames } from "./child- import { UndiciHeaders, UndiciRequest, UndiciResponse } from "./undici.js"; import { BUFFER_CONSTANTS, BUFFER_MAX_LENGTH, BUFFER_MAX_STRING_LENGTH } from "./buffer-constants.js"; import { EventEmitter, once } from "./events.js"; -import { _fs, _processCpuUsage, _processMemoryUsage, _processResourceUsage, _processUmask, _processVersions, decodeBridgeJson, normalizeModeArgument } from "./fs.js"; +import { _processCpuUsage, _processMemoryUsage, _processResourceUsage, _processUmask, _processVersions, bridgeErrorCode, createBridgeSyncFacade, createFsError, normalizeModeArgument } from "./fs.js"; import { builtinPathStdlibModule } from "./builtin-modules.js"; import { fetch } from "./network.js"; import { getRuntimeGid, getRuntimeUid } from "./os.js"; @@ -434,6 +434,13 @@ hrtime.bigint = function() { }; var _cwd = config2.cwd; +// The kernel owns the cwd; `_cwd` caches its answer for `process.cwd()`. +var _processChdir = createBridgeSyncFacade("process.chdir"); +var CHDIR_ERROR_DESCRIPTIONS = { + EACCES: "permission denied", + ENOENT: "no such file or directory", + ENOTDIR: "not a directory" +}; var _umask = 18; @@ -608,28 +615,18 @@ var process2 = { cwd() { return _cwd; }, - chdir(dir) { - let statJson; + chdir(directory) { try { - statJson = _fs.stat.applySyncPromise(void 0, [dir]); - } catch { - const err = new Error(`ENOENT: no such file or directory, chdir '${dir}'`); - err.code = "ENOENT"; - err.errno = -2; - err.syscall = "chdir"; - err.path = dir; - throw err; - } - const parsed = decodeBridgeJson(statJson); - if (!parsed.isDirectory) { - const err = new Error(`ENOTDIR: not a directory, chdir '${dir}'`); - err.code = "ENOTDIR"; - err.errno = -20; - err.syscall = "chdir"; - err.path = dir; - throw err; + _cwd = _processChdir.applySync(void 0, [directory]); + } catch (error) { + const code = bridgeErrorCode(error); + const description = CHDIR_ERROR_DESCRIPTIONS[code]; + if (!description) throw error; + // Node's uv error shape: `path` is the old cwd, `dest` the argument. + const chdirError = createFsError(code, `${code}: ${description}, chdir '${_cwd}' -> '${directory}'`, "chdir", _cwd); + chdirError.dest = directory; + throw chdirError; } - _cwd = dir; }, get exitCode() { return _exitCode; diff --git a/packages/build-tools/bridge-src/global-exposure.ts b/packages/build-tools/bridge-src/global-exposure.ts index be2b279b51..b4e4582e4b 100644 --- a/packages/build-tools/bridge-src/global-exposure.ts +++ b/packages/build-tools/bridge-src/global-exposure.ts @@ -32,6 +32,11 @@ var NODE_CUSTOM_GLOBAL_INVENTORY = [ classification: "hardened", rationale: "Host process resource usage bridge reference.", }, + { + name: "process.chdir", + classification: "hardened", + rationale: "Kernel-owned guest filesystem and process bridge reference.", + }, { name: "process.umask", classification: "hardened", diff --git a/packages/runtime-browser/scripts/check-bridge-contract.mjs b/packages/runtime-browser/scripts/check-bridge-contract.mjs index 83383b47b9..a3d379e554 100644 --- a/packages/runtime-browser/scripts/check-bridge-contract.mjs +++ b/packages/runtime-browser/scripts/check-bridge-contract.mjs @@ -110,6 +110,7 @@ const browserFacadeContractGlobals = new Set([ "fs.openSync", "fs.readSync", "fs.writeSync", + "process.chdir", "process.cpuUsage", "process.memoryUsage", "process.resourceUsage", From 03ca715c65f2ee1f9110d096f3db30e83fb78147 Mon Sep 17 00:00:00 2001 From: Sree Narayanan Date: Wed, 21 Oct 2026 13:41:28 +0400 Subject: [PATCH 2/2] fix(runtime): match Node symlink and directory handling in fs cp, readdir and rm cp copies symlinks as links (honoring dereference and verbatimSymlinks) and applies the filter option, readdir Dirents report symlinks without following them, and rm on a directory without recursive throws ERR_FS_EISDIR. The sidecar readdir payload now carries the entry's libuv dirent type (file, directory or symlink) instead of a followed is-directory flag. Callback rm reuses rmSync instead of a second tree walk. --- crates/native-sidecar/src/filesystem.rs | 120 ++++++------ .../tests/builtin_conformance.rs | 166 +++++++++++++++++ crates/native-sidecar/tests/service.rs | 3 +- .../build-tools/bridge-src/builtins/fs.ts | 175 ++++++++++-------- 4 files changed, 328 insertions(+), 136 deletions(-) diff --git a/crates/native-sidecar/src/filesystem.rs b/crates/native-sidecar/src/filesystem.rs index 68bf0c256c..54de0bebf4 100644 --- a/crates/native-sidecar/src/filesystem.rs +++ b/crates/native-sidecar/src/filesystem.rs @@ -46,8 +46,8 @@ use agentos_execution::{ }; use agentos_kernel::kernel::is_internal_unnamed_file_name; use agentos_kernel::vfs::{ - VirtualFileSystem, VirtualStat, VirtualTimeSpec, VirtualUtimeSpec, RENAME_EXCHANGE, - RENAME_NOREPLACE, + VirtualDirEntry, VirtualFileSystem, VirtualStat, VirtualTimeSpec, VirtualUtimeSpec, + RENAME_EXCHANGE, RENAME_NOREPLACE, }; use agentos_native_sidecar_core::{ decode_guest_filesystem_content, handle_guest_filesystem_call as core_guest_filesystem_call, @@ -5036,37 +5036,40 @@ fn remove_dest_at(dst_dir: BorrowedFd<'_>, name: &std::ffi::OsStr) -> std::io::R } } -fn mapped_readdir_entry_is_directory( - mapped_host: &MappedRuntimeHostPath, - directory: &MappedRuntimeOpenedPath, - guest_dir_path: &str, - name: &std::ffi::OsStr, - kind: crate::plugins::host_dir::confine::EntryKind, -) -> Option { - match kind { - crate::plugins::host_dir::confine::EntryKind::Directory => Some(true), - crate::plugins::host_dir::confine::EntryKind::Other => Some(false), - // A symlink entry is followed by re-resolving it beneath the same root - // (fd-anchored), then classifying the target via `fstat`. - crate::plugins::host_dir::confine::EntryKind::Symlink => { - let name_str = name.to_str()?; - let child = MappedRuntimeHostPath { - guest_path: normalize_path(&format!( - "{}/{}", - guest_dir_path.trim_end_matches('/'), - name_str - )), - host_root: mapped_host.host_root.clone(), - host_path: directory.host_path.join(name), - }; - let opened = open_mapped_runtime_beneath( - &child, - "fs.readdir entry", - O_PATH_ANCHOR, - Mode::empty(), - ) - .ok()?; - opened.handle.metadata().map(|stat| stat.is_directory).ok() +/// Dirent kind sent to the guest `fs` bridge. Like Node's `readdir`, entries +/// are typed without following symlinks. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum JavascriptDirentKind { + File, + Directory, + Symlink, +} + +impl JavascriptDirentKind { + /// libuv `uv_dirent_type_t` values, which the bridge's `Dirent` uses as is. + fn wire_code(self) -> u8 { + match self { + Self::File => 1, + Self::Directory => 2, + Self::Symlink => 3, + } + } + + fn from_kernel_entry(entry: &VirtualDirEntry) -> Self { + if entry.is_symbolic_link { + Self::Symlink + } else if entry.is_directory { + Self::Directory + } else { + Self::File + } + } + + fn from_mapped_entry(kind: crate::plugins::host_dir::confine::EntryKind) -> Self { + match kind { + crate::plugins::host_dir::confine::EntryKind::Directory => Self::Directory, + crate::plugins::host_dir::confine::EntryKind::Symlink => Self::Symlink, + crate::plugins::host_dir::confine::EntryKind::Other => Self::File, } } } @@ -5076,11 +5079,11 @@ pub(crate) fn service_javascript_fs_readdir_entries( process: &ActiveProcess, kernel_pid: u32, path: &str, -) -> Result, SidecarError> { +) -> Result, SidecarError> { if let Some(MappedRuntimeHostAccess::Writable(mapped_host)) = mapped_runtime_host_path(kernel, process, path, false) { - let mut typed: BTreeMap = BTreeMap::new(); + let mut typed: BTreeMap = BTreeMap::new(); match open_mapped_runtime_beneath( &mapped_host, "fs.readdir", @@ -5099,18 +5102,10 @@ pub(crate) fn service_javascript_fs_readdir_entries( )) })?; for (name, kind) in entries { - if let Some(is_dir) = mapped_readdir_entry_is_directory( - &mapped_host, - &directory, - path, - &name, - kind, - ) { - let Ok(name) = name.into_string() else { - continue; - }; - typed.insert(name, is_dir); - } + let Ok(name) = name.into_string() else { + continue; + }; + typed.insert(name, JavascriptDirentKind::from_mapped_entry(kind)); } } // The host dir simply not existing yet is fine — fall through to the @@ -5127,14 +5122,15 @@ pub(crate) fn service_javascript_fs_readdir_entries( match kernel.read_dir_with_types_for_process(EXECUTION_DRIVER_NAME, kernel_pid, path) { Ok(entries) => { for entry in entries { - typed.entry(entry.name).or_insert(entry.is_directory); + let kind = JavascriptDirentKind::from_kernel_entry(&entry); + typed.entry(entry.name).or_insert(kind); } } Err(error) if matches!(error.code(), "ENOENT" | "ENOTDIR") => {} Err(error) => return Err(kernel_error(error)), } for name in mapped_runtime_child_mount_basenames(process, path) { - typed.entry(name).or_insert(true); + typed.entry(name).or_insert(JavascriptDirentKind::Directory); } return Ok(typed); } @@ -5144,7 +5140,10 @@ pub(crate) fn service_javascript_fs_readdir_entries( .map(|entries| { entries .into_iter() - .map(|entry| (entry.name, entry.is_directory)) + .map(|entry| { + let kind = JavascriptDirentKind::from_kernel_entry(&entry); + (entry.name, kind) + }) .collect() }) .map_err(kernel_error) @@ -5164,10 +5163,10 @@ pub(crate) fn service_javascript_fs_readdir_raw_sync_rpc( } fn encode_javascript_readdir_raw_payload( - entries: BTreeMap, + entries: BTreeMap, ) -> Result, SidecarError> { let mut payload = Vec::new(); - for (name, is_dir) in entries + for (name, kind) in entries .into_iter() .filter(|(name, _)| name != "." && name != "..") { @@ -5175,22 +5174,25 @@ fn encode_javascript_readdir_raw_payload( let name_len = u32::try_from(name.len()).map_err(|_| { SidecarError::InvalidState(String::from("filesystem readdir entry name too long")) })?; - payload.push(u8::from(is_dir)); + payload.push(kind.wire_code()); payload.extend_from_slice(&name_len.to_le_bytes()); payload.extend_from_slice(&name); } Ok(payload) } -/// Like `javascript_sync_rpc_readdir_value` but carries each entry's -/// directory-ness as `{name, isDirectory}`. The guest's `normalizeReaddirEntries` -/// consumes these objects directly for `withFileTypes`, avoiding a per-entry stat -/// RPC, and extracts `.name` for the plain string form. -fn javascript_sync_rpc_readdir_typed_value(entries: BTreeMap) -> Value { +/// Like `javascript_sync_rpc_readdir_value` but carries each entry's Dirent +/// kind as `{name, kind}`, using the raw payload's kind codes. The guest's +/// `normalizeReaddirEntries` consumes these objects directly for +/// `withFileTypes`, avoiding a per-entry stat RPC, and extracts `.name` for the +/// plain string form. +fn javascript_sync_rpc_readdir_typed_value( + entries: BTreeMap, +) -> Value { json!(entries .into_iter() .filter(|(name, _)| name != "." && name != "..") - .map(|(name, is_dir)| json!({ "name": name, "isDirectory": is_dir })) + .map(|(name, kind)| json!({ "name": name, "kind": kind.wire_code() })) .collect::>()) } diff --git a/crates/native-sidecar/tests/builtin_conformance.rs b/crates/native-sidecar/tests/builtin_conformance.rs index c3c49e46c6..a02e0306aa 100644 --- a/crates/native-sidecar/tests/builtin_conformance.rs +++ b/crates/native-sidecar/tests/builtin_conformance.rs @@ -2424,6 +2424,167 @@ fn process_chdir_moves_relative_paths_and_children_matches_host_node() { run_isolated_builtin_conformance_test("process-chdir-cwd"); } +// Each walker case runs under the host-mapped cwd and under a /tmp directory, +// which the guest serves from the kernel VFS. The host probe shares the cwd, +// so each run uses fresh directories and removes them. +const FS_TWO_ROOTS_PRELUDE: &str = r#" +import fs from "node:fs"; +import path from "node:path"; +const roots = [ + fs.mkdtempSync(path.join(process.cwd(), "case-")), + fs.mkdtempSync("/tmp/agentos-conformance-"), +]; +const attempt = async (fn) => { + try { + await fn(); + return null; + } catch (error) { + return error.code ?? error.message; + } +}; +const runInRoots = async (run) => { + const results = []; + for (const root of roots) { + results.push(await run(root)); + fs.rmSync(root, { recursive: true }); + } + console.log(JSON.stringify(results)); +}; +"#; + +fn cp_copies_symlinks_as_links_matches_host_node_impl() { + let script = format!( + "{FS_TWO_ROOTS_PRELUDE}{}", + r#" +await runInRoots(async (root) => { + const shown = (target) => (target.startsWith(root) ? `${target.slice(root.length)}` : target); + const entry = (path) => { + const stats = fs.lstatSync(path); + if (stats.isSymbolicLink()) return { link: shown(fs.readlinkSync(path)) }; + if (stats.isDirectory()) return { dir: fs.readdirSync(path).sort() }; + return { file: fs.readFileSync(path, "utf8") }; + }; + const tree = (dir) => + Object.fromEntries(fs.readdirSync(dir).sort().map((name) => [name, entry(`${dir}/${name}`)])); + + fs.mkdirSync(`${root}/keep`); + fs.writeFileSync(`${root}/keep/data.txt`, "keep"); + fs.mkdirSync(`${root}/src`); + fs.writeFileSync(`${root}/src/file.txt`, "file"); + fs.symlinkSync("../keep", `${root}/src/rel-dir`); + fs.symlinkSync(`${root}/keep/data.txt`, `${root}/src/abs-file`); + fs.symlinkSync("missing", `${root}/src/dangling`); + fs.mkdirSync(`${root}/existing`); + + const result = { + intoExisting: await attempt(() => fs.cpSync(`${root}/src`, `${root}/existing`, { recursive: true })), + existing: tree(`${root}/existing`), + verbatim: await attempt(() => + fs.promises.cp(`${root}/src`, `${root}/verbatim`, { recursive: true, verbatimSymlinks: true }) + ), + verbatimTree: tree(`${root}/verbatim`), + topLevelLinkWithoutRecursive: await attempt(() => fs.cpSync(`${root}/src/rel-dir`, `${root}/top-link`)), + topLevelLink: await attempt(() => + fs.cpSync(`${root}/src/rel-dir`, `${root}/top-link`, { recursive: true }) + ), + topLevel: entry(`${root}/top-link`), + topLevelDangling: await attempt(() => fs.cpSync(`${root}/src/dangling`, `${root}/top-dangling`)), + }; + result.dereference = await attempt(() => + fs.cpSync(`${root}/src/rel-dir`, `${root}/deref`, { recursive: true, dereference: true }) + ); + result.dereferenced = entry(`${root}/deref`); + result.keep = fs.readdirSync(`${root}/keep`); + return result; +}); +"# + ); + assert_conformance("fs-cp-symlinks", &script); +} + +#[test] +fn cp_copies_symlinks_as_links_matches_host_node() { + run_isolated_builtin_conformance_test("fs-cp-symlinks"); +} + +fn readdir_dirent_types_match_host_node_impl() { + let script = format!( + "{FS_TWO_ROOTS_PRELUDE}{}", + r#" +await runInRoots(async (root) => { + fs.mkdirSync(`${root}/dir/sub`, { recursive: true }); + fs.writeFileSync(`${root}/dir/file.txt`, "file"); + fs.symlinkSync("sub", `${root}/dir/link-dir`); + fs.symlinkSync("file.txt", `${root}/dir/link-file`); + fs.symlinkSync("missing", `${root}/dir/dangling`); + const types = (entries) => + entries + .map((dirent) => [ + dirent.name, + dirent.isFile(), + dirent.isDirectory(), + dirent.isSymbolicLink(), + dirent.isFIFO(), + dirent.isSocket(), + dirent.isCharacterDevice(), + dirent.isBlockDevice(), + ]) + .sort(); + return { + sync: types(fs.readdirSync(`${root}/dir`, { withFileTypes: true })), + promises: types(await fs.promises.readdir(`${root}/dir`, { withFileTypes: true })), + }; +}); +"# + ); + assert_conformance("fs-dirent-types", &script); +} + +#[test] +fn readdir_dirent_types_match_host_node() { + run_isolated_builtin_conformance_test("fs-dirent-types"); +} + +fn rm_directory_without_recursive_matches_host_node_impl() { + assert_conformance( + "fs-rm-directory-without-recursive", + r#" +import fs from "node:fs"; + +fs.mkdirSync("full"); +fs.writeFileSync("full/file.txt", "full"); +fs.mkdirSync("empty"); +const describe = async (remove) => { + try { + await remove(); + return null; + } catch (error) { + const { code, name, errno, syscall, message } = error; + return { code, name, errno, syscall, message }; + } +}; + +console.log( + JSON.stringify({ + full: await describe(() => fs.rmSync("full")), + empty: await describe(() => fs.rmSync("empty")), + force: await describe(() => fs.rmSync("empty", { force: true })), + callback: await describe( + () => new Promise((resolve, reject) => fs.rm("empty", (error) => (error ? reject(error) : resolve()))) + ), + promises: await describe(() => fs.promises.rm("empty")), + remaining: ["full", "full/file.txt", "empty"].filter((path) => fs.existsSync(path)), + }) +); +"#, + ); +} + +#[test] +fn rm_directory_without_recursive_matches_host_node() { + run_isolated_builtin_conformance_test("fs-rm-directory-without-recursive"); +} + fn console_conformance_matches_host_node() { assert_conformance( "console", @@ -5200,6 +5361,11 @@ fn __builtin_conformance_extra_test_runner() { "process-chdir-cwd" => { process_chdir_moves_relative_paths_and_children_matches_host_node_impl() } + "fs-cp-symlinks" => cp_copies_symlinks_as_links_matches_host_node_impl(), + "fs-dirent-types" => readdir_dirent_types_match_host_node_impl(), + "fs-rm-directory-without-recursive" => { + rm_directory_without_recursive_matches_host_node_impl() + } other => panic!("unknown builtin conformance extra test: {other}"), } } diff --git a/crates/native-sidecar/tests/service.rs b/crates/native-sidecar/tests/service.rs index 743afc0eca..ac8ad0a440 100644 --- a/crates/native-sidecar/tests/service.rs +++ b/crates/native-sidecar/tests/service.rs @@ -17030,7 +17030,8 @@ process.stdout.write(`${JSON.stringify({ plain, typed, empty })}\n`); assert_eq!(by_name("empty")["isDirectory"], json!(true)); assert_eq!(by_name("file.txt")["isDirectory"], json!(false)); assert_eq!(by_name("kernel.txt")["isDirectory"], json!(false)); - assert_eq!(by_name("link-dir")["isDirectory"], json!(true)); + // Like Node, a Dirent describes the link itself, not its target. + assert_eq!(by_name("link-dir")["isDirectory"], json!(false)); assert_eq!(by_name("link-file")["isDirectory"], json!(false)); for entry in typed { assert_eq!( diff --git a/packages/build-tools/bridge-src/builtins/fs.ts b/packages/build-tools/bridge-src/builtins/fs.ts index 1bef434b51..09e7ce5862 100644 --- a/packages/build-tools/bridge-src/builtins/fs.ts +++ b/packages/build-tools/bridge-src/builtins/fs.ts @@ -1,6 +1,7 @@ import { deferCloseIfChildInheritedFd } from "./child-process.js"; import { builtinCryptoModule } from "./crypto.js"; import { _umask } from "./process.js"; +import { builtinPathStdlibModule } from "./builtin-modules.js"; import { clearTimeout2, setTimeout2 } from "./timers.js"; import { exposeCustomGlobal } from "../global-exposure.js"; import { require_buffer } from "../vendor/buffer.js"; @@ -79,26 +80,38 @@ var Stats = class { return (this.mode & 61440) === 49152; } }; +// Dirent types use libuv's `uv_dirent_type_t` numbers, as Node does. The +// sidecar readdir payload sends the same codes. +const UV_DIRENT_UNKNOWN = 0; +const UV_DIRENT_FILE = 1; +const UV_DIRENT_DIR = 2; +const UV_DIRENT_LINK = 3; +function direntTypeFromStats(stats) { + if (stats.isFile()) return UV_DIRENT_FILE; + if (stats.isDirectory()) return UV_DIRENT_DIR; + if (stats.isSymbolicLink()) return UV_DIRENT_LINK; + return UV_DIRENT_UNKNOWN; +} var Dirent = class { name; parentPath; path; // Deprecated alias for parentPath - _isDir; - constructor(name, isDir, parentPath = "") { + _type; + constructor(name, type, parentPath = "") { this.name = name; - this._isDir = isDir; + this._type = type; this.parentPath = parentPath; this.path = parentPath; } isFile() { - return !this._isDir; + return this._type === UV_DIRENT_FILE; } isDirectory() { - return this._isDir; + return this._type === UV_DIRENT_DIR; } isSymbolicLink() { - return false; + return this._type === UV_DIRENT_LINK; } isBlockDevice() { return false; @@ -2010,6 +2023,65 @@ function createFsError(code, message, syscall, path) { if (path) err.path = path; return err; } +// Like Node, `filter(src, dest)` runs before the pair is validated, and a false +// result skips that path and everything under it. +function cpFilterSkips(srcPath, destPath, opts) { + if (!opts.filter) return false; + const shouldCopy = opts.filter(srcPath, destPath); + if (typeof shouldCopy?.then === "function") { + const error = new TypeError( + 'Expected boolean to be returned from the "filter" function but got instance of Promise.' + ); + error.code = "ERR_INVALID_RETURN_VALUE"; + throw error; + } + return !shouldCopy; +} +function copyPathSync(srcPath, destPath, opts) { + const srcStat = opts.dereference ? fs.statSync(srcPath) : fs.lstatSync(srcPath); + if (srcStat.isSymbolicLink()) { + // Node's cp copies a link as a link. Unless `verbatimSymlinks` is set, a + // relative target is made absolute against the source link's directory. + let target = fs.readlinkSync(srcPath); + if (!opts.verbatimSymlinks && !target.startsWith("/")) { + target = builtinPathStdlibModule.resolve(builtinPathStdlibModule.dirname(srcPath), target); + } + fs.symlinkSync(target, destPath); + } else if (srcStat.isDirectory()) { + fs.mkdirSync(destPath, { recursive: true }); + for (const entry of fs.readdirSync(srcPath)) { + const srcEntry = joinDirEntryPath(srcPath, entry); + const destEntry = joinDirEntryPath(destPath, entry); + if (!cpFilterSkips(srcEntry, destEntry, opts)) { + copyPathSync(srcEntry, destEntry, opts); + } + } + } else { + if (opts.errorOnExist && fs.existsSync(destPath)) { + throw createFsError( + "EEXIST", + `EEXIST: file already exists, cp '${srcPath}' -> '${destPath}'`, + "cp", + destPath + ); + } + if (!opts.force && opts.force !== void 0 && fs.existsSync(destPath)) { + return; + } + fs.copyFileSync(srcPath, destPath); + } +} +function createRmIsDirectoryError(path) { + const err = createFsError( + "ERR_FS_EISDIR", + `Path is a directory: rm returned EISDIR (is a directory) ${path}`, + "rm", + path + ); + err.name = "SystemError"; + err.errno = POSIX_ERRNO.EISDIR; + return err; +} function bridgeErrorText(err) { return String(err?.message ?? err ?? ""); } @@ -2160,7 +2232,7 @@ function _globCollect(pattern, options, results) { const name = patternBase.split("/").filter(Boolean).pop() ?? patternBase; const lastSlash = scanBase.lastIndexOf("/"); const parent = lastSlash < 0 ? "." : lastSlash === 0 ? "/" : scanBase.slice(0, lastSlash); - const entry = new Dirent(name, stat.isDirectory(), parent); + const entry = new Dirent(name, direntTypeFromStats(stat), parent); if (!_globExcludeDecision(patternBase, entry, options).excluded) { addResult(patternBase, options.withFileTypes ? entry : patternBase); } @@ -2368,10 +2440,10 @@ function normalizeReaddirEntries(entries, dirPath, withFileTypes) { } return entries.map((entry) => { if (typeof entry === "string") { - const stat = fs.statSync(joinDirEntryPath(dirPath, entry)); - return new Dirent(entry, stat.isDirectory(), dirPath); + const stat = fs.lstatSync(joinDirEntryPath(dirPath, entry)); + return new Dirent(entry, direntTypeFromStats(stat), dirPath); } - return new Dirent(entry.name, entry.isDirectory, dirPath); + return new Dirent(entry.name, entry.kind, dirPath); }); } function decodeRawReaddirEntries(entries, dirPath, withFileTypes) { @@ -2396,7 +2468,7 @@ function decodeRawReaddirEntries(entries, dirPath, withFileTypes) { nameLength ).toString("utf8"); offset += nameLength; - decoded.push(withFileTypes ? new Dirent(name, kind === 1, dirPath) : name); + decoded.push(withFileTypes ? new Dirent(name, kind, dirPath) : name); } return decoded; } @@ -2846,7 +2918,7 @@ var fs = { } fs.rmdirSync(pathStr); } else { - fs.rmdirSync(pathStr); + throw createRmIsDirectoryError(pathStr); } } else { fs.unlinkSync(pathStr); @@ -2935,40 +3007,20 @@ var fs = { const srcPath = toPathString(src); const destPath = toPathString(dest); const opts = options || {}; - const srcStat = fs.statSync(srcPath); - if (srcStat.isDirectory()) { - if (!opts.recursive) { - throw createFsError( - "ERR_FS_EISDIR", - `Path is a directory: cp '${srcPath}'`, - "cp", - srcPath - ); - } - try { - fs.mkdirSync(destPath, { recursive: true }); - } catch { - } - const entries = fs.readdirSync(srcPath); - for (const entry of entries) { - const srcEntry = srcPath.endsWith("/") ? srcPath + entry : srcPath + "/" + entry; - const destEntry = destPath.endsWith("/") ? destPath + entry : destPath + "/" + entry; - fs.cpSync(srcEntry, destEntry, opts); - } - } else { - if (opts.errorOnExist && fs.existsSync(destPath)) { - throw createFsError( - "EEXIST", - `EEXIST: file already exists, cp '${srcPath}' -> '${destPath}'`, - "cp", - destPath - ); - } - if (!opts.force && opts.force !== void 0 && fs.existsSync(destPath)) { - return; - } - fs.copyFileSync(srcPath, destPath); + if (cpFilterSkips(srcPath, destPath, opts)) { + return; + } + // Like Node, the top-level source check follows a symlink: a dangling + // source fails and a linked directory still needs `recursive`. + if (fs.statSync(srcPath).isDirectory() && !opts.recursive) { + throw createFsError( + "ERR_FS_EISDIR", + `Path is a directory: cp '${srcPath}'`, + "cp", + srcPath + ); } + copyPathSync(srcPath, destPath, opts); }, // Temp directory creation mkdtempSync(prefix, _options) { @@ -3424,44 +3476,15 @@ var fs = { } else { cb = callback; } - const doRm = () => { - try { - const stats = fs.lstatSync(path); - if (stats.isDirectory()) { - if (opts.recursive) { - const entries = fs.readdirSync(path); - for (const entry of entries) { - const entryPath = path.endsWith("/") ? path + entry : path + "/" + entry; - const entryStats = fs.lstatSync(entryPath); - if (entryStats.isDirectory()) { - fs.rmSync(entryPath, { recursive: true }); - } else { - fs.unlinkSync(entryPath); - } - } - fs.rmdirSync(path); - } else { - fs.rmdirSync(path); - } - } else { - fs.unlinkSync(path); - } - } catch (e) { - if (opts.force && e.code === "ENOENT") { - return; - } - throw e; - } - }; if (cb) { try { - doRm(); + fs.rmSync(path, opts); queueMicrotask(() => cb(null)); } catch (e) { queueMicrotask(() => cb(e)); } } else { - doRm(); + fs.rmSync(path, opts); return Promise.resolve(); } },