From 4076651b9ddf6be4aac91262b8c34e46ce73878a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 13 Nov 2026 08:22:33 +0000 Subject: [PATCH 1/2] Keep YarnEnv unit tests off the host's yarn env YarnEnv::current reads YARN_NODE_LINKER, YARN_PNPM_STORE_FOLDER, YARN_RC_FILENAME and YARN_INJECT_ENVIRONMENT_FILES from the process env and the rc file in the home folder. #2054 moved the detector tests onto a hermetic variant, but every crawler, vendor and vex path below it still built YarnEnv from the host, so on origin/main: - YARN_NODE_LINKER=node-modules fails 2 npm_crawler tests (yarn 4 pnpm-linker relocated store); - HOME with a .yarnrc.yml setting nodeLinker: node-modules fails 10 vendor tests (lock_inventory, npm_flavor, refusals, yarn_berry_lock). Under cfg(test), read only the YARN_* vars and HOME/USERPROFILE a live EnvGuard sets, the same way utils::go_env reads Go vars through test_env::guarded_var. Production builds (and the CLI integration tests, which link the non-test core) keep reading the real env. Refs #1919. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011f9aeSopYBBeiUVSewPUqN --- .../src/crawlers/pkg_managers.rs | 78 +++++++++++++++++-- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/pkg_managers.rs b/crates/socket-patch-core/src/crawlers/pkg_managers.rs index 404434443..e3b8f642e 100644 --- a/crates/socket-patch-core/src/crawlers/pkg_managers.rs +++ b/crates/socket-patch-core/src/crawlers/pkg_managers.rs @@ -223,11 +223,11 @@ impl YarnEnv { // Yarn takes the last `yarn_node_linker` spelling in the // environment (each one overwrites the setting) but the first // `yarn_rc_filename` (`getRcFilename` returns on a match). - node_linker: yarn_env_key(std::env::vars_os(), "YARN_NODE_LINKER", true), - pnpm_store_folder: yarn_env_key(std::env::vars_os(), "YARN_PNPM_STORE_FOLDER", true), - rc_filename: yarn_env_key(std::env::vars_os(), "YARN_RC_FILENAME", false) + node_linker: yarn_env_key(yarn_process_vars(), "YARN_NODE_LINKER", true), + pnpm_store_folder: yarn_env_key(yarn_process_vars(), "YARN_PNPM_STORE_FOLDER", true), + rc_filename: yarn_env_key(yarn_process_vars(), "YARN_RC_FILENAME", false) .unwrap_or_else(|| ".yarnrc.yml".to_string()), - home: crate::utils::fs::home_dir(), + home: yarn_home(), vars: None, env_files: Default::default(), scope, @@ -240,7 +240,7 @@ impl YarnEnv { scope: crate::utils::repo_root::ReadScope, project_root: &Path, ) -> Self { - let env_list = yarn_env_key(std::env::vars_os(), "YARN_INJECT_ENVIRONMENT_FILES", true); + let env_list = yarn_env_key(yarn_process_vars(), "YARN_INJECT_ENVIRONMENT_FILES", true); Self::current(scope).with_env_files(project_root, env_list.as_deref()) } @@ -360,6 +360,41 @@ impl YarnEnv { } } +/// The home folder whose rc file [`YarnEnv::current`] reads last. +#[cfg(not(test))] +fn yarn_home() -> Option { + crate::utils::fs::home_dir() +} + +/// In unit tests, only a `HOME`/`USERPROFILE` a live `EnvGuard` sets: a +/// developer's `~/.yarnrc.yml` can't change what the tests see (#1919). +#[cfg(test)] +fn yarn_home() -> Option { + crate::utils::fs::home_from_env(|key| crate::test_env::guarded_var(key).map(Into::into)) +} + +/// The process environment the `YARN_*` lookups of [`YarnEnv::current`] +/// and [`YarnEnv::for_project`] read. +#[cfg(not(test))] +fn yarn_process_vars() -> std::env::VarsOs { + std::env::vars_os() +} + +/// In unit tests, only the variables a live `EnvGuard` holds: a +/// developer's `YARN_NODE_LINKER` (or `YARN_PNPM_STORE_FOLDER`, +/// `YARN_RC_FILENAME`, `YARN_INJECT_ENVIRONMENT_FILES`) can't change what +/// the crawler tests see (#1919), as `utils::go_env` does for Go. +#[cfg(test)] +fn yarn_process_vars() -> std::vec::IntoIter<(std::ffi::OsString, std::ffi::OsString)> { + std::env::vars_os() + .filter(|(key, _)| { + key.to_str() + .is_some_and(|key| crate::test_env::guarded_var(key).is_some()) + }) + .collect::>() + .into_iter() +} + /// The yarn environment setting `key` (upper case) in `vars`: yarn /// lower-cases every environment key before matching it, so /// `yarn_node_linker` sets the linker as `YARN_NODE_LINKER` does (#1100; @@ -704,6 +739,39 @@ impl YarnPnpLoader { mod tests { use super::*; + /// The unit tests read a `YARN_*` variable or the home folder only + /// while a guard sets it, so a developer's `YARN_NODE_LINKER` or + /// `~/.yarnrc.yml` can't flip a test (#1919), while a test that sets + /// one still sees it. + #[test] + #[serial_test::serial] + fn yarn_env_sees_only_guarded_variables() { + use crate::utils::repo_root::ReadScope; + let home = tempfile::tempdir().unwrap(); + { + let _linker = crate::test_env::EnvGuard::many([ + ( + "YARN_NODE_LINKER", + Some(std::ffi::OsStr::new("node-modules")), + ), + ( + "YARN_PNPM_STORE_FOLDER", + Some(std::ffi::OsStr::new("store")), + ), + ("HOME", Some(home.path().as_os_str())), + ]); + let env = YarnEnv::current(ReadScope::Workspace); + assert_eq!(env.node_linker.as_deref(), Some("node-modules")); + assert_eq!(env.pnpm_store_folder.as_deref(), Some("store")); + assert_eq!(env.home.as_deref(), Some(home.path())); + } + let env = YarnEnv::current(ReadScope::Workspace); + assert_eq!(env.node_linker, None); + assert_eq!(env.home, None); + assert_eq!(env.pnpm_store_folder, None); + assert_eq!(env.rc_filename, ".yarnrc.yml"); + } + /// The public entry points only add the host's yarn env, which is /// asked for a loader file alone: without one they agree with the /// hermetic variants the other tests use. From 3007a1c36be8c6394d37fd7607e5ace0b256b254 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 13 Nov 2026 08:32:54 +0000 Subject: [PATCH 2/2] Scope YarnEnv's ${NAME} lookups to guards too Review follow-ups on the YarnEnv test seam: - base_var fell back to std::env::var under cfg(test), so a host variable named in a fixture's ${NAME} reference still leaked. It now reads only guarded variables in unit tests. - Windows keeps a key's existing spelling when a guard sets it in another case, so match the upper-case spelling there too. - Read the process env once per YarnEnv::current, not three times. - Cover YARN_RC_FILENAME and the ${NAME} lookup in the new test. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011f9aeSopYBBeiUVSewPUqN --- .../src/crawlers/pkg_managers.rs | 56 ++++++++++++++----- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/pkg_managers.rs b/crates/socket-patch-core/src/crawlers/pkg_managers.rs index e3b8f642e..3aa4f4491 100644 --- a/crates/socket-patch-core/src/crawlers/pkg_managers.rs +++ b/crates/socket-patch-core/src/crawlers/pkg_managers.rs @@ -219,13 +219,14 @@ pub(crate) struct EnvFiles { impl YarnEnv { pub(crate) fn current(scope: crate::utils::repo_root::ReadScope) -> Self { + let vars = yarn_process_vars(); Self { // Yarn takes the last `yarn_node_linker` spelling in the // environment (each one overwrites the setting) but the first // `yarn_rc_filename` (`getRcFilename` returns on a match). - node_linker: yarn_env_key(yarn_process_vars(), "YARN_NODE_LINKER", true), - pnpm_store_folder: yarn_env_key(yarn_process_vars(), "YARN_PNPM_STORE_FOLDER", true), - rc_filename: yarn_env_key(yarn_process_vars(), "YARN_RC_FILENAME", false) + node_linker: yarn_env_key(vars.iter().cloned(), "YARN_NODE_LINKER", true), + pnpm_store_folder: yarn_env_key(vars.iter().cloned(), "YARN_PNPM_STORE_FOLDER", true), + rc_filename: yarn_env_key(vars.iter().cloned(), "YARN_RC_FILENAME", false) .unwrap_or_else(|| ".yarnrc.yml".to_string()), home: yarn_home(), vars: None, @@ -240,7 +241,11 @@ impl YarnEnv { scope: crate::utils::repo_root::ReadScope, project_root: &Path, ) -> Self { - let env_list = yarn_env_key(yarn_process_vars(), "YARN_INJECT_ENVIRONMENT_FILES", true); + let env_list = yarn_env_key( + yarn_process_vars().into_iter(), + "YARN_INJECT_ENVIRONMENT_FILES", + true, + ); Self::current(scope).with_env_files(project_root, env_list.as_deref()) } @@ -331,7 +336,7 @@ impl YarnEnv { fn base_var(&self, name: &str) -> Option { match &self.vars { Some(vars) => vars.get(name).cloned(), - None => std::env::var(name).ok(), + None => yarn_process_var(name), } } @@ -376,23 +381,38 @@ fn yarn_home() -> Option { /// The process environment the `YARN_*` lookups of [`YarnEnv::current`] /// and [`YarnEnv::for_project`] read. #[cfg(not(test))] -fn yarn_process_vars() -> std::env::VarsOs { - std::env::vars_os() +fn yarn_process_vars() -> Vec<(std::ffi::OsString, std::ffi::OsString)> { + std::env::vars_os().collect() } /// In unit tests, only the variables a live `EnvGuard` holds: a /// developer's `YARN_NODE_LINKER` (or `YARN_PNPM_STORE_FOLDER`, /// `YARN_RC_FILENAME`, `YARN_INJECT_ENVIRONMENT_FILES`) can't change what -/// the crawler tests see (#1919), as `utils::go_env` does for Go. +/// the tests see (#1919), as `utils::go_env` does for Go. Windows keeps a +/// key's existing spelling when a guard sets it in another case, so there +/// the upper-case spelling a guard uses counts too. #[cfg(test)] -fn yarn_process_vars() -> std::vec::IntoIter<(std::ffi::OsString, std::ffi::OsString)> { +fn yarn_process_vars() -> Vec<(std::ffi::OsString, std::ffi::OsString)> { + let held = |key: &str| { + crate::test_env::guarded_var(key).is_some() + || (cfg!(windows) && crate::test_env::guarded_var(&key.to_ascii_uppercase()).is_some()) + }; std::env::vars_os() - .filter(|(key, _)| { - key.to_str() - .is_some_and(|key| crate::test_env::guarded_var(key).is_some()) - }) - .collect::>() - .into_iter() + .filter(|(key, _)| key.to_str().is_some_and(held)) + .collect() +} + +/// The variable a `${NAME}` reference in a yarn rc or env file expands +/// from when [`YarnEnv::vars`] is `None`. +#[cfg(not(test))] +fn yarn_process_var(name: &str) -> Option { + std::env::var(name).ok() +} + +/// In unit tests, only a variable a live `EnvGuard` sets (#1919). +#[cfg(test)] +fn yarn_process_var(name: &str) -> Option { + crate::test_env::guarded_var(name) } /// The yarn environment setting `key` (upper case) in `vars`: yarn @@ -758,18 +778,24 @@ mod tests { "YARN_PNPM_STORE_FOLDER", Some(std::ffi::OsStr::new("store")), ), + ("YARN_RC_FILENAME", Some(std::ffi::OsStr::new(".alt.yml"))), + ("SOCKET_YARN_TEST_VAR", Some(std::ffi::OsStr::new("x"))), ("HOME", Some(home.path().as_os_str())), ]); let env = YarnEnv::current(ReadScope::Workspace); assert_eq!(env.node_linker.as_deref(), Some("node-modules")); assert_eq!(env.pnpm_store_folder.as_deref(), Some("store")); + assert_eq!(env.rc_filename, ".alt.yml"); assert_eq!(env.home.as_deref(), Some(home.path())); + assert_eq!(env.var("SOCKET_YARN_TEST_VAR").as_deref(), Some("x")); } let env = YarnEnv::current(ReadScope::Workspace); assert_eq!(env.node_linker, None); assert_eq!(env.home, None); assert_eq!(env.pnpm_store_folder, None); assert_eq!(env.rc_filename, ".yarnrc.yml"); + assert_eq!(env.var("SOCKET_YARN_TEST_VAR"), None); + assert_eq!(env.var("PATH"), None, "an unguarded host variable"); } /// The public entry points only add the host's yarn env, which is