From 960aba343db5d037bcdb0e4be45863319d2e00a1 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Mon, 27 Jul 2026 23:52:32 +0200 Subject: [PATCH] fix(executor): preserve inherited TMPDIR (#13445) Pacquet's shared script environment builder treated TMPDIR as an always-reserved stamp. Explicit pnpm run calls use unsafePerm, so no replacement was written and the child lost the user-provided temporary directory entirely. Preserve inherited TMPDIR by default and continue replacing it with the per-package node_modules/.tmp directory when unsafePerm is disabled. This matches the TypeScript CLI and normal child-process environment inheritance. Closes pnpm/pnpm#13442. --- .changeset/pacquet-preserve-tmpdir.md | 5 +++ pnpm/crates/cli/tests/run.rs | 42 ++++++++++++++++++++++ pnpm/crates/executor/src/make_env.rs | 33 ++++++++++++----- pnpm/crates/executor/src/make_env/tests.rs | 33 +++++++++++++---- 4 files changed, 97 insertions(+), 16 deletions(-) create mode 100644 .changeset/pacquet-preserve-tmpdir.md diff --git a/.changeset/pacquet-preserve-tmpdir.md b/.changeset/pacquet-preserve-tmpdir.md new file mode 100644 index 0000000000..fddc52ceea --- /dev/null +++ b/.changeset/pacquet-preserve-tmpdir.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +Preserve a user-provided `TMPDIR` when scripts run with `unsafePerm` enabled; otherwise, continue using the package-local temporary directory. diff --git a/pnpm/crates/cli/tests/run.rs b/pnpm/crates/cli/tests/run.rs index 19d1fb042a..8123e8edb7 100644 --- a/pnpm/crates/cli/tests/run.rs +++ b/pnpm/crates/cli/tests/run.rs @@ -374,6 +374,48 @@ fn run_preserves_embedded_quotes_in_script() { drop(root); } +#[test] +fn run_preserves_parent_tmpdir() { + let CommandTempCwd { pacquet, root, workspace, .. } = CommandTempCwd::init(); + let alternate_tmpdir = workspace.join("project-tmp"); + fs::create_dir(&alternate_tmpdir).expect("create alternate temp dir"); + fs::write( + workspace.join("show-tmp.js"), + "require('fs').writeFileSync('tmpdir.json', JSON.stringify({ \ +env: process.env.TMPDIR, os: require('os').tmpdir() }))", + ) + .expect("write show-tmp.js"); + fs::write( + workspace.join("package.json"), + json!({ + "name": "test", + "version": "0.0.0", + "scripts": { "show-tmp": "node show-tmp.js" }, + }) + .to_string(), + ) + .expect("write package.json"); + + pacquet + .with_env("TMPDIR", &alternate_tmpdir) + .with_arg("run") + .with_arg("show-tmp") + .assert() + .success(); + + let recorded: serde_json::Value = serde_json::from_str( + &fs::read_to_string(workspace.join("tmpdir.json")).expect("read tmpdir.json"), + ) + .expect("parse tmpdir.json"); + let expected_tmpdir = alternate_tmpdir.to_string_lossy(); + assert_eq!(recorded["env"], expected_tmpdir.as_ref()); + if cfg!(not(windows)) { + assert_eq!(recorded["os"], expected_tmpdir.as_ref()); + } + + drop(root); +} + /// A failing `test` script prints pnpm's stage-specific lifecycle error /// (`Test failed. See above for more details.`) rather than the generic /// exit-code line, matching reportLifecycleError's `test` special case. diff --git a/pnpm/crates/executor/src/make_env.rs b/pnpm/crates/executor/src/make_env.rs index 991559e1be..df330fef83 100644 --- a/pnpm/crates/executor/src/make_env.rs +++ b/pnpm/crates/executor/src/make_env.rs @@ -54,19 +54,29 @@ pub struct EnvBuild { /// `parent_env` is taken by value so the production caller can pass /// `env::vars().collect()` and tests can pass a controlled fixture /// without racing on the global process env. +#[must_use] pub fn build_env( opts: &EnvOptions<'_>, manifest: &Value, parent_env: HashMap, +) -> EnvBuild { + build_env_for_platform(opts, manifest, parent_env, cfg!(windows)) +} + +fn build_env_for_platform( + opts: &EnvOptions<'_>, + manifest: &Value, + parent_env: HashMap, + is_windows: bool, ) -> EnvBuild { // 1. Start from the parent env, stripping `npm_package_*` (we // regenerate them below) and the `(npm|pnpm)_config_*` auth // keys, plus the per-call stamps we re-derive (`NODE`, - // `TMPDIR`, `INIT_CWD`, `PNPM_SCRIPT_SRC_DIR`). User-defined + // `INIT_CWD`, `PNPM_SCRIPT_SRC_DIR`). User-defined // `npm_config_*` such as `npm_config_platform_arch` are // preserved. `pnpm_*` keys such as `PNPM_HOME` are intentionally // NOT in the filter. - let mut env = filter_parent_env(parent_env); + let mut env = filter_parent_env(parent_env, is_windows); // 2. `npm_package_*` recursive stamp. Top-level keeps only // name/version/config/engines/bin; recursion below those @@ -118,7 +128,7 @@ pub fn build_env( // the same casing rule that filter uses, since on Windows a // differently-cased entry names the same variable. for (k, v) in opts.extra_env { - if is_dev_preinstall_marker(k, cfg!(windows)) { + if is_dev_preinstall_marker(k, is_windows) { continue; } env.insert(k.clone(), v.clone()); @@ -143,6 +153,11 @@ pub fn build_env( None } else { let dir = opts.pkg_root.join("node_modules").join(".tmp"); + // Windows treats differently cased spellings as one variable, + // so remove them before inserting the authoritative override. + if is_windows { + env.retain(|key, _| !key.eq_ignore_ascii_case("TMPDIR")); + } env.insert("TMPDIR".into(), dir.to_string_lossy().into_owned()); Some(dir) }; @@ -164,14 +179,14 @@ pub fn build_env( /// an unpredictable winner. /// /// [`Command::env`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.env -fn filter_parent_env(env: HashMap) -> HashMap { - env.into_iter().filter(|(k, _)| !is_stamping_key(k, cfg!(windows))).collect() +fn filter_parent_env(env: HashMap, is_windows: bool) -> HashMap { + env.into_iter().filter(|(k, _)| !is_stamping_key(k, is_windows)).collect() } /// Whether `key` must be dropped from the inherited parent env: an /// `npm_package_*` stamp, a `(npm|pnpm)_config_*` auth credential, a -/// per-call stamp [`build_env`] re-derives (`NODE`, `TMPDIR`, -/// `INIT_CWD`, `PNPM_SCRIPT_SRC_DIR`), or +/// per-call stamp [`build_env`] re-derives (`NODE`, `INIT_CWD`, +/// `PNPM_SCRIPT_SRC_DIR`), or /// [`DEV_PREINSTALL_ALREADY_RAN_ENV`]. Stripping the auth credentials /// keeps them out of dependency lifecycle scripts; stripping the /// delegation marker keeps it scoped to the install that received it, @@ -190,8 +205,8 @@ fn is_stamping_key(key: &str, is_windows: bool) -> bool { { return true; } - const DROPPED: [&str; 5] = - ["NODE", "TMPDIR", "INIT_CWD", "PNPM_SCRIPT_SRC_DIR", DEV_PREINSTALL_ALREADY_RAN_ENV]; + const DROPPED: [&str; 4] = + ["NODE", "INIT_CWD", "PNPM_SCRIPT_SRC_DIR", DEV_PREINSTALL_ALREADY_RAN_ENV]; if is_windows { return DROPPED.iter().any(|name| key.eq_ignore_ascii_case(name)); } diff --git a/pnpm/crates/executor/src/make_env/tests.rs b/pnpm/crates/executor/src/make_env/tests.rs index 2ee93951ff..3f3662ec52 100644 --- a/pnpm/crates/executor/src/make_env/tests.rs +++ b/pnpm/crates/executor/src/make_env/tests.rs @@ -1,6 +1,7 @@ use super::{ DEV_PREINSTALL_ALREADY_RAN_ENV, EnvOptions, VERIFY_DEPS_BEFORE_RUN_ENV, build_env, - escape_newlines, is_dev_preinstall_marker, is_stamping_key, sanitize_env_key, stamp_package, + build_env_for_platform, escape_newlines, is_dev_preinstall_marker, is_stamping_key, + sanitize_env_key, stamp_package, }; use pretty_assertions::assert_eq; use serde_json::json; @@ -157,23 +158,41 @@ fn make_env_stamps_lifecycle_specific_keys() { } #[test] -fn make_env_tmpdir_gating_mirrors_unsafe_perm() { +fn make_env_preserves_or_overrides_tmpdir_based_on_unsafe_perm() { let pkg_root = Path::new("/tmp/z"); let extra = empty_extra(); + let parent = HashMap::from([("TMPDIR".to_string(), "/alternate/tmp".to_string())]); let mut opts = base_opts(pkg_root, pkg_root, &extra); opts.unsafe_perm = true; - let built = build_env(&opts, &json!({"name":"z","version":"0"}), HashMap::new()); + let built = build_env(&opts, &json!({"name":"z","version":"0"}), parent.clone()); assert!(built.tmpdir.is_none()); - assert!(!built.env.contains_key("TMPDIR")); + assert_eq!(built.env.get("TMPDIR").map(String::as_str), Some("/alternate/tmp")); opts.unsafe_perm = false; - let built = build_env(&opts, &json!({"name":"z","version":"0"}), HashMap::new()); + let built = build_env(&opts, &json!({"name":"z","version":"0"}), parent); let expected_tmpdir = pkg_root.join("node_modules").join(".tmp"); assert_eq!(built.tmpdir.as_deref(), Some(expected_tmpdir.as_path())); assert_eq!(built.env.get("TMPDIR"), Some(&expected_tmpdir.to_string_lossy().into_owned())); } +#[test] +fn make_env_windows_tmpdir_override_removes_differently_cased_keys() { + let pkg_root = Path::new("/tmp/z"); + let extra = HashMap::from([("tmpdir".to_string(), "/extra/tmp".to_string())]); + let parent = HashMap::from([("TmpDir".to_string(), "/parent/tmp".to_string())]); + let mut opts = base_opts(pkg_root, pkg_root, &extra); + opts.unsafe_perm = false; + + let built = build_env_for_platform(&opts, &json!({"name":"z","version":"0"}), parent, true); + let expected_tmpdir = pkg_root.join("node_modules").join(".tmp"); + + assert_eq!(built.env.get("TMPDIR"), Some(&expected_tmpdir.to_string_lossy().into_owned())); + let tmpdir_key_count = + built.env.keys().filter(|key| key.eq_ignore_ascii_case("TMPDIR")).count(); + assert_eq!(tmpdir_key_count, 1); +} + /// pnpm's reserved per-call stamps override a user `extraEnv` that /// tries to set the same key (matching TS `runLifecycleHook`, which /// spreads `extraEnv` before its own `INIT_CWD` / user-agent / etc.). @@ -287,7 +306,7 @@ fn is_stamping_key_is_case_sensitive_on_posix() { assert!(is_stamping_key("NODE", false)); assert!(!is_stamping_key("Node", false)); assert!(!is_stamping_key("node", false)); - assert!(is_stamping_key("TMPDIR", false)); + assert!(!is_stamping_key("TMPDIR", false)); assert!(is_stamping_key("INIT_CWD", false)); assert!(is_stamping_key("PNPM_SCRIPT_SRC_DIR", false)); assert!(!is_stamping_key("PNPM_HOME", false)); @@ -368,7 +387,7 @@ fn is_stamping_key_is_case_insensitive_on_windows() { assert!(is_stamping_key("NODE", true)); assert!(is_stamping_key("Node", true)); assert!(is_stamping_key("node", true)); - assert!(is_stamping_key("tmpdir", true)); + assert!(!is_stamping_key("tmpdir", true)); assert!(is_stamping_key("init_cwd", true)); assert!(is_stamping_key("pnpm_script_src_dir", true)); assert!(!is_stamping_key("NPM", true));