Skip to content

Commit 01ea663

Browse files
divybotlittledivy
andauthored
fix(ext/process): tolerate unlinked cwd in spawn (#33587)
## Summary Enables `test-cwd-enoent-repl` in node_compat suite. ## Test plan - [x] `cargo test --test node_compat -- test-cwd-enoent-repl` --------- Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 9c23f62 commit 01ea663

4 files changed

Lines changed: 120 additions & 17 deletions

File tree

cli/factory.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,20 @@ impl CliFactory {
721721
self.services.workspace_factory.get_or_try_init(|| {
722722
let initial_cwd = match self.overrides.initial_cwd.clone() {
723723
Some(v) => v,
724+
// For modes that don't depend on a real cwd (REPL, eval), fall back
725+
// to a sentinel path when current_dir() fails — matches Node.js
726+
// semantics where `node --interactive` works even after the cwd has
727+
// been unlinked.
728+
None
729+
if matches!(
730+
self.flags.subcommand,
731+
DenoSubcommand::Repl(_) | DenoSubcommand::Eval(_)
732+
) =>
733+
{
734+
crate::util::env::resolve_cwd_or_fallback(
735+
self.flags.initial_cwd.as_deref(),
736+
)
737+
}
724738
None => {
725739
crate::util::env::resolve_cwd(self.flags.initial_cwd.as_deref())?
726740
.into_owned()

cli/util/env.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::collections::HashSet;
66
use std::env;
77
use std::ffi::OsString;
88
use std::path::Path;
9+
use std::path::PathBuf;
910
use std::sync::Arc;
1011
use std::sync::Mutex;
1112
use std::sync::OnceLock;
@@ -32,6 +33,32 @@ pub fn resolve_cwd(
3233
}
3334
}
3435

36+
/// Like `resolve_cwd`, but falls back to a sensible default (the system
37+
/// root) when the current working directory can't be determined — for
38+
/// example when it has been unlinked. This matches Node.js semantics where
39+
/// the REPL still starts even if the parent process's cwd was deleted.
40+
pub fn resolve_cwd_or_fallback(initial_cwd: Option<&Path>) -> PathBuf {
41+
match resolve_cwd(initial_cwd) {
42+
Ok(cwd) => cwd.into_owned(),
43+
Err(_) => fallback_cwd(),
44+
}
45+
}
46+
47+
fn fallback_cwd() -> PathBuf {
48+
if cfg!(windows) {
49+
// System drive root, e.g. `C:\`.
50+
std::env::var_os("SystemDrive")
51+
.map(|d| {
52+
let mut p = PathBuf::from(d);
53+
p.push("\\");
54+
p
55+
})
56+
.unwrap_or_else(|| PathBuf::from("C:\\"))
57+
} else {
58+
PathBuf::from("/")
59+
}
60+
}
61+
3562
#[derive(Debug, Clone)]
3663
struct WatchEnvTrackerInner {
3764
// Track all loaded variables and their values

ext/process/lib.rs

Lines changed: 78 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,7 @@ fn create_command(
532532
state: &mut OpState,
533533
mut args: SpawnArgs,
534534
api_name: &str,
535+
allow_cwd_inherit: bool,
535536
) -> Result<CreateCommand, ProcessError> {
536537
let maybe_npm_process_state = if args.needs_npm_process_state {
537538
let provider = state.borrow::<NpmProcessStateProviderRc>();
@@ -553,6 +554,7 @@ fn create_command(
553554
args.clear_env,
554555
state,
555556
api_name,
557+
allow_cwd_inherit,
556558
)?;
557559
let mut command = Command::new(cmd);
558560

@@ -576,7 +578,9 @@ fn create_command(
576578
command.args(args.args);
577579
}
578580

579-
command.current_dir(run_env.cwd);
581+
if run_env.set_cwd_on_command {
582+
command.current_dir(&run_env.cwd);
583+
}
580584
command.env_clear();
581585
command.envs(run_env.envs.into_iter().map(|(k, v)| (k.into_inner(), v)));
582586

@@ -1059,14 +1063,14 @@ fn compute_run_cmd_and_check_permissions(
10591063
arg_clear_env: bool,
10601064
state: &mut OpState,
10611065
api_name: &str,
1066+
allow_cwd_inherit: bool,
10621067
) -> Result<(PathBuf, RunEnv), ProcessError> {
10631068
let run_env =
1064-
compute_run_env(arg_cwd, arg_envs, arg_clear_env).map_err(|e| {
1065-
ProcessError::SpawnFailed {
1069+
compute_run_env(arg_cwd, arg_envs, arg_clear_env, allow_cwd_inherit)
1070+
.map_err(|e| ProcessError::SpawnFailed {
10661071
command: arg_cmd.to_string(),
10671072
error: Box::new(e),
1068-
}
1069-
})?;
1073+
})?;
10701074
let cmd =
10711075
resolve_cmd(arg_cmd, &run_env).map_err(|e| ProcessError::SpawnFailed {
10721076
command: arg_cmd.to_string(),
@@ -1138,28 +1142,57 @@ impl std::cmp::PartialEq for EnvVarKey {
11381142

11391143
struct RunEnv {
11401144
envs: HashMap<EnvVarKey, OsString>,
1145+
/// Best-effort cwd used for resolving relative cmd paths and PATH lookups.
1146+
/// When the cwd cannot be determined and the caller permits inheritance
1147+
/// (see `set_cwd_on_command`), this is set to `"."` as a placeholder.
11411148
cwd: PathBuf,
1149+
/// When `false`, the spawned `Command` should not have its cwd set
1150+
/// explicitly so that the child inherits the parent's (possibly unlinked)
1151+
/// cwd. This matches Node.js semantics for `child_process.spawn` and is
1152+
/// only enabled for Node-compat code paths.
1153+
set_cwd_on_command: bool,
11421154
}
11431155

11441156
/// Computes the current environment, which will then be used to inform
11451157
/// permissions and finally spawning. This is very important to compute
11461158
/// ahead of time so that the environment used to verify permissions is
11471159
/// the same environment used to spawn the sub command. This protects against
11481160
/// someone doing timing attacks by changing the environment on a worker.
1161+
///
1162+
/// `allow_cwd_inherit` controls whether spawning is allowed to proceed when
1163+
/// no explicit cwd was passed and `current_dir()` fails (e.g. the parent's
1164+
/// cwd has been unlinked). Only Node-compat ops opt into this; Deno's own
1165+
/// `Deno.run` / `Deno.Command` keep the existing strict behavior.
11491166
fn compute_run_env(
11501167
arg_cwd: Option<&str>,
11511168
arg_envs: &[(String, String)],
11521169
arg_clear_env: bool,
1170+
allow_cwd_inherit: bool,
11531171
) -> Result<RunEnv, ProcessError> {
11541172
#[allow(
11551173
clippy::disallowed_methods,
11561174
reason = "ok for now because launching a sub process requires the real fs"
11571175
)]
1158-
let cwd =
1159-
std::env::current_dir().map_err(ProcessError::FailedResolvingCwd)?;
1160-
let cwd = arg_cwd
1161-
.map(|cwd_arg| resolve_path(cwd_arg, &cwd))
1162-
.unwrap_or(cwd);
1176+
let current_dir = std::env::current_dir();
1177+
let (cwd, set_cwd_on_command) = match arg_cwd {
1178+
Some(cwd_arg) => {
1179+
let arg_path = Path::new(cwd_arg);
1180+
if arg_path.is_absolute() {
1181+
(
1182+
deno_path_util::normalize_path(Cow::Borrowed(arg_path)).into_owned(),
1183+
true,
1184+
)
1185+
} else {
1186+
let base = current_dir.map_err(ProcessError::FailedResolvingCwd)?;
1187+
(resolve_path(cwd_arg, &base), true)
1188+
}
1189+
}
1190+
None => match current_dir {
1191+
Ok(c) => (c, true),
1192+
Err(_) if allow_cwd_inherit => (PathBuf::from("."), false),
1193+
Err(e) => return Err(ProcessError::FailedResolvingCwd(e)),
1194+
},
1195+
};
11631196
let envs = if arg_clear_env {
11641197
arg_envs
11651198
.iter()
@@ -1174,17 +1207,36 @@ fn compute_run_env(
11741207
}
11751208
envs
11761209
};
1177-
Ok(RunEnv { envs, cwd })
1210+
Ok(RunEnv {
1211+
envs,
1212+
cwd,
1213+
set_cwd_on_command,
1214+
})
11781215
}
11791216

11801217
fn resolve_cmd(cmd: &str, env: &RunEnv) -> Result<PathBuf, ProcessError> {
11811218
let is_path = cmd.contains('/');
11821219
#[cfg(windows)]
11831220
let is_path = is_path || cmd.contains('\\') || Path::new(&cmd).is_absolute();
11841221
if is_path {
1185-
Ok(resolve_path(cmd, &env.cwd))
1222+
let cmd_path = Path::new(cmd);
1223+
if cmd_path.is_absolute() {
1224+
Ok(deno_path_util::normalize_path(Cow::Borrowed(cmd_path)).into_owned())
1225+
} else if !env.set_cwd_on_command {
1226+
// Relative cmd path can't be resolved without a known cwd.
1227+
Err(ProcessError::FailedResolvingCwd(std::io::Error::from(
1228+
std::io::ErrorKind::NotFound,
1229+
)))
1230+
} else {
1231+
Ok(resolve_path(cmd, &env.cwd))
1232+
}
11861233
} else {
11871234
let path = env.envs.get(&EnvVarKey::new(OsString::from("PATH")));
1235+
// When the cwd is unknown (`set_cwd_on_command == false`) `env.cwd` is
1236+
// a placeholder `"."`. PATH-resolvable names don't need a real cwd; for
1237+
// unqualified names that fall back to a cwd-relative search this is a
1238+
// best-effort lookup that will simply miss when the parent's cwd has
1239+
// been unlinked.
11881240
match deno_permissions::which::which_in(
11891241
sys_traits::impls::RealSys,
11901242
cmd,
@@ -1291,7 +1343,7 @@ fn op_spawn_child(
12911343
) -> Result<Child, ProcessError> {
12921344
let detached = args.detached;
12931345
let (command, pipe_rid, extra_pipe_fds, handles_to_close) =
1294-
create_command(state, args, &api_name)?;
1346+
create_command(state, args, &api_name, /* allow_cwd_inherit */ false)?;
12951347
let child = spawn_child(state, command, pipe_rid, extra_pipe_fds, detached);
12961348
for handle in handles_to_close {
12971349
deno_io::close_raw_handle(handle);
@@ -1308,8 +1360,10 @@ fn op_node_spawn_child(
13081360
#[string] api_name: String,
13091361
) -> Result<NodeChild, ProcessError> {
13101362
let detached = args.detached;
1363+
// `child_process.spawn` in Node tolerates the parent's cwd being unlinked
1364+
// by inheriting it, so allow cwd inheritance for Node-compat spawns.
13111365
let (command, pipe_rid, extra_pipe_fds, handles_to_close) =
1312-
create_command(state, args, &api_name)?;
1366+
create_command(state, args, &api_name, /* allow_cwd_inherit */ true)?;
13131367
let child =
13141368
spawn_child_node(state, command, pipe_rid, extra_pipe_fds, detached);
13151369
for handle in handles_to_close {
@@ -1356,8 +1410,12 @@ fn op_spawn_sync(
13561410
let timeout = args.timeout;
13571411
#[cfg(unix)]
13581412
let kill_signal = args.kill_signal.clone();
1359-
let (mut command, _, _, _) =
1360-
create_command(state, args, "Deno.Command().outputSync()")?;
1413+
let (mut command, _, _, _) = create_command(
1414+
state,
1415+
args,
1416+
"Deno.Command().outputSync()",
1417+
/* allow_cwd_inherit */ false,
1418+
)?;
13611419

13621420
// When timeout is specified on Unix, create a new process group so we can
13631421
// kill the entire tree (shell + children) on timeout, not just the shell.
@@ -1656,6 +1714,7 @@ mod deprecated {
16561714
/* clear env */ false,
16571715
state,
16581716
"Deno.run()",
1717+
/* allow_cwd_inherit */ false,
16591718
)?;
16601719

16611720
#[cfg(windows)]
@@ -1665,7 +1724,9 @@ mod deprecated {
16651724
for arg in args.iter().skip(1) {
16661725
c.arg(arg);
16671726
}
1668-
c.current_dir(run_env.cwd);
1727+
if run_env.set_cwd_on_command {
1728+
c.current_dir(&run_env.cwd);
1729+
}
16691730

16701731
c.env_clear();
16711732
for (key, value) in run_env.envs {

tests/node_compat/config.jsonc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,7 @@
697697
"parallel/test-crypto-webcrypto-aes-decrypt-tag-too-small.js": {},
698698
"parallel/test-crypto-worker-thread.js": {},
699699
"parallel/test-crypto-x509.js": {},
700+
"parallel/test-cwd-enoent-repl.js": {},
700701
"parallel/test-datetime-change-notify.js": {},
701702
"parallel/test-debugger-address.mjs": {
702703
"ignore": true,

0 commit comments

Comments
 (0)