From 64588da2d9a051dbd396eeb40e48bdbbc8fc2c98 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 28 Oct 2026 01:31:28 +0100 Subject: [PATCH] fix(workspace-state): keep the state file readable to other users (#17438) The workspace state file was written through `private_named_tempfile_in`, so `node_modules/.pnpm-workspace-state-v1.json` ended up 0600 while pnpm 11 left it 0644 (`fs.writeFile`). Another user could not read it, so the deps-status check before `pnpm run` treated `node_modules` as stale and reinstalled. A Docker image that installs as root and runs as a non-root user hits this on every start. Add `pnpm_fs::umask_named_tempfile_in`, which creates the temp file with 0666 narrowed by the umask, and use it for the state file. The write stays without an fsync, as before. --- .changeset/workspace-state-file-mode.md | 5 +++++ pnpm/crates/cli/tests/suite/umask.rs | 11 +++++++++ pnpm/crates/fs/src/lib.rs | 2 ++ pnpm/crates/fs/src/umask_named_tempfile.rs | 26 ++++++++++++++++++++++ pnpm/crates/workspace-state/src/lib.rs | 2 +- pnpm/crates/workspace-state/src/tests.rs | 23 +++++++++++++++++++ 6 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 .changeset/workspace-state-file-mode.md create mode 100644 pnpm/crates/fs/src/umask_named_tempfile.rs diff --git a/.changeset/workspace-state-file-mode.md b/.changeset/workspace-state-file-mode.md new file mode 100644 index 0000000000..a131c90789 --- /dev/null +++ b/.changeset/workspace-state-file-mode.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +`pnpm install` now creates `node_modules/.pnpm-workspace-state-v1.json` with the same permissions as other new files. It was readable only by the user who ran the install, so a different user running `pnpm run` reinstalled the dependencies. This happened, for example, in a Docker image built as root and run as a non-root user [#17435](https://github.com/pnpm/pnpm/discussions/17435). diff --git a/pnpm/crates/cli/tests/suite/umask.rs b/pnpm/crates/cli/tests/suite/umask.rs index 48bdb40b2c..650342acf2 100644 --- a/pnpm/crates/cli/tests/suite/umask.rs +++ b/pnpm/crates/cli/tests/suite/umask.rs @@ -12,6 +12,7 @@ use pnpm_testing_utils::{ bin::{AddMockedRegistry, CommandTempCwd}, command_env::CommandTestExt, }; +use pnpm_workspace_state::WORKSPACE_STATE_FILENAME; use std::{ fs, os::unix::{fs::PermissionsExt, process::CommandExt}, @@ -116,6 +117,16 @@ fn installs_materialize_files_at_the_current_umask_even_from_a_stale_store() { 0o644, "the store link matches the umask that wrote the store", ); + assert_eq!( + file_mode(&workspace.join("node_modules").join(WORKSPACE_STATE_FILENAME)), + 0o644, + "the workspace state file follows the install's umask", + ); + assert_eq!( + file_mode(&other.join("node_modules").join(WORKSPACE_STATE_FILENAME)), + 0o600, + "the workspace state file follows the install's umask", + ); use std::os::unix::fs::MetadataExt; let inode = |path: &Path| fs::metadata(path).expect("stat").ino(); diff --git a/pnpm/crates/fs/src/lib.rs b/pnpm/crates/fs/src/lib.rs index 814250dc3d..b5d985a094 100644 --- a/pnpm/crates/fs/src/lib.rs +++ b/pnpm/crates/fs/src/lib.rs @@ -48,6 +48,7 @@ pub use secure_temp_lock::{ }; pub use symlink_dir::*; pub use sync_file::sync_file; +pub use umask_named_tempfile::umask_named_tempfile_in; #[cfg(target_os = "wasi")] pub use wasi_fs::{ check_file_owner, create_inheriting_mode as create_file_inheriting_mode, @@ -85,6 +86,7 @@ mod retry; mod secure_temp_lock; mod symlink_dir; mod sync_file; +mod umask_named_tempfile; #[cfg(target_os = "wasi")] mod wasi_fs; mod write_atomic; diff --git a/pnpm/crates/fs/src/umask_named_tempfile.rs b/pnpm/crates/fs/src/umask_named_tempfile.rs new file mode 100644 index 0000000000..3fc24bbbe6 --- /dev/null +++ b/pnpm/crates/fs/src/umask_named_tempfile.rs @@ -0,0 +1,26 @@ +use std::{io, path::Path}; + +/// Creates a named temporary file with the mode a new file from `fs::write` gets: +/// 0666 narrowed by the umask on Unix and WASI. +/// +/// For files other users may need to read, such as a `node_modules` built by root +/// in one Docker stage and used by another user in the next. +/// [`private_named_tempfile_in`](crate::private_named_tempfile_in) is the 0600 counterpart. +#[inline] +pub fn umask_named_tempfile_in(directory: impl AsRef) -> io::Result { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + tempfile::Builder::new() + .permissions(std::fs::Permissions::from_mode(0o666)) + .tempfile_in(directory) + } + #[cfg(target_os = "wasi")] + { + tempfile::Builder::new().make_in(directory, |path| crate::create_new_with_mode(path, 0o666)) + } + #[cfg(not(any(unix, target_os = "wasi")))] + { + tempfile::NamedTempFile::new_in(directory) + } +} diff --git a/pnpm/crates/workspace-state/src/lib.rs b/pnpm/crates/workspace-state/src/lib.rs index 13ae448bf2..b7b1949384 100644 --- a/pnpm/crates/workspace-state/src/lib.rs +++ b/pnpm/crates/workspace-state/src/lib.rs @@ -436,7 +436,7 @@ pub fn update_workspace_state_in_modules( serde_json::to_string_pretty(state).map_err(UpdateWorkspaceStateError::SerializeJson)?; serialized.push('\n'); let write = |source| UpdateWorkspaceStateError::WriteFile { path: file_path.clone(), source }; - let mut temp = pnpm_fs::private_named_tempfile_in(parent).map_err(write)?; + let mut temp = pnpm_fs::umask_named_tempfile_in(parent).map_err(write)?; temp.write_all(serialized.as_bytes()).map_err(write)?; let temp = temp.into_temp_path(); pnpm_fs::rename_with_retry(&temp, &file_path).map_err(write) diff --git a/pnpm/crates/workspace-state/src/tests.rs b/pnpm/crates/workspace-state/src/tests.rs index 07fca21a37..85e2ca6621 100644 --- a/pnpm/crates/workspace-state/src/tests.rs +++ b/pnpm/crates/workspace-state/src/tests.rs @@ -248,6 +248,29 @@ fn update_surfaces_write_file_error_when_target_is_a_directory() { assert_eq!(node_modules_entries(workspace_dir), vec![WORKSPACE_STATE_FILENAME.to_string()]); } +/// Another user must be able to read the state file, for example when root +/// installs in one Docker stage and an unprivileged user runs scripts in the next. +#[cfg(unix)] +#[test] +fn update_gives_the_state_file_the_mode_of_any_new_file() { + use std::os::unix::fs::PermissionsExt as _; + let tmp = tempdir().expect("create temp dir"); + let workspace_dir = tmp.path(); + let reference = workspace_dir.join("reference"); + std::fs::File::create(&reference).expect("create reference file"); + + update_workspace_state(workspace_dir, &WorkspaceState::default()).expect("write state"); + + let mode_of = |path: &std::path::Path| { + std::fs::metadata(path) + .expect("stat file") + .permissions() + .mode() + & 0o777 + }; + assert_eq!(mode_of(&get_file_path(workspace_dir)), mode_of(&reference)); +} + #[test] fn update_leaves_no_temp_file_beside_the_state_file() { let tmp = tempdir().expect("create temp dir");