diff --git a/pacquet/crates/package-manifest/Cargo.toml b/pacquet/crates/package-manifest/Cargo.toml index abb4d139d4..1e14d8208f 100644 --- a/pacquet/crates/package-manifest/Cargo.toml +++ b/pacquet/crates/package-manifest/Cargo.toml @@ -16,6 +16,7 @@ miette = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } strum = { workspace = true } +tempfile = { workspace = true } [dev-dependencies] pipe-trait = { workspace = true } diff --git a/pacquet/crates/package-manifest/src/lib.rs b/pacquet/crates/package-manifest/src/lib.rs index aff769518e..e13fbc477a 100644 --- a/pacquet/crates/package-manifest/src/lib.rs +++ b/pacquet/crates/package-manifest/src/lib.rs @@ -9,6 +9,7 @@ use miette::Diagnostic; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value, json}; use strum::IntoStaticStr; +use tempfile::NamedTempFile; #[derive(Debug, Display, Error, Diagnostic, From)] #[non_exhaustive] @@ -96,6 +97,28 @@ impl PackageManifest { Ok((manifest, contents)) } + /// Write `contents` to `path` atomically: a sibling temp file is written + /// and fsynced, then renamed over `path`. A crash or write error therefore + /// never leaves a truncated or partial `package.json` behind, matching the + /// `write-file-atomic` guarantee the TypeScript pnpm CLI relies on. + fn write_atomic(path: &Path, contents: &str) -> io::Result<()> { + let dir = path + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let mut tmp = NamedTempFile::new_in(dir)?; + tmp.write_all(contents.as_bytes())?; + tmp.as_file().sync_all()?; + // A NamedTempFile is created 0o600; preserve the original file's mode + // when overwriting an existing package.json (write-file-atomic does the + // same) so the rename doesn't silently tighten its permissions. + if let Ok(metadata) = fs::metadata(path) { + tmp.as_file().set_permissions(metadata.permissions())?; + } + tmp.persist(path).map_err(|err| err.error)?; + Ok(()) + } + fn read_from_file(path: &Path) -> Result { let contents = fs::read_to_string(path)?; let mut value: Value = serde_json::from_str(&contents)?; @@ -158,8 +181,7 @@ impl PackageManifest { convert_dependencies_to_engines_runtime(&mut value, "devDependencies", "devEngines")?; convert_dependencies_to_engines_runtime(&mut value, "dependencies", "engines")?; let contents = serde_json::to_string_pretty(&value)?; - let mut file = fs::File::create(&self.path)?; - file.write_all(contents.as_bytes())?; + Self::write_atomic(&self.path, &contents)?; Ok(value) } diff --git a/pacquet/crates/package-manifest/src/tests.rs b/pacquet/crates/package-manifest/src/tests.rs index 586281a788..17fe69b67e 100644 --- a/pacquet/crates/package-manifest/src/tests.rs +++ b/pacquet/crates/package-manifest/src/tests.rs @@ -15,6 +15,50 @@ use crate::DependencyGroup; use serde_json::json; use std::io::Write; +#[cfg(unix)] +#[test] +fn save_leaves_the_original_intact_when_the_write_cannot_complete() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempdir().unwrap(); + let path = dir.path().join("package.json"); + let original = r#"{"name":"intact","version":"1.0.0"}"#; + std::fs::write(&path, original).unwrap(); + + let manifest = PackageManifest::from_path(path.clone()).unwrap(); + + // Make the directory read-only so a sibling temp file cannot be created. + // An atomic temp-file-then-rename write fails up front and leaves the + // original untouched; a non-atomic in-place write would instead truncate + // the existing package.json before failing. + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o555)).unwrap(); + let result = manifest.save(); + // Restore permissions before the assertions so tempdir cleanup succeeds. + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + + assert!(result.is_err()); + assert_eq!(std::fs::read_to_string(&path).unwrap(), original); +} + +#[cfg(unix)] +#[test] +fn save_preserves_the_existing_package_json_permissions() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempdir().unwrap(); + let path = dir.path().join("package.json"); + std::fs::write(&path, r#"{"name":"perm","version":"1.0.0"}"#).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o640)).unwrap(); + + let manifest = PackageManifest::from_path(path.clone()).unwrap(); + manifest.save().unwrap(); + + // The atomic temp-file-then-rename must keep the original mode, not leave + // the NamedTempFile's default 0o600 behind. + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o640); +} + #[test] fn test_init_package_json_content() { let manifest = PackageManifest::create_init_package_json("test");