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.
This commit is contained in:
1 parent
c59e2a4c7b
commit
960aba343d
4 files changed
+97
-16
No files matched your search
@@ -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.
|
||||
@@ -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.
|
||||
|
||||
@@ -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<String, String>,
|
||||
) -> EnvBuild {
|
||||
build_env_for_platform(opts, manifest, parent_env, cfg!(windows))
|
||||
}
|
||||
|
||||
fn build_env_for_platform(
|
||||
opts: &EnvOptions<'_>,
|
||||
manifest: &Value,
|
||||
parent_env: HashMap<String, String>,
|
||||
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<String, String>) -> HashMap<String, String> {
|
||||
env.into_iter().filter(|(k, _)| !is_stamping_key(k, cfg!(windows))).collect()
|
||||
fn filter_parent_env(env: HashMap<String, String>, is_windows: bool) -> HashMap<String, String> {
|
||||
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));
|
||||
}
|
||||
|
||||
@@ -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));
|
||||
|
||||
Reference in new issue
Block a user