diff --git a/.changeset/fix-workspace-state-race.md b/.changeset/fix-workspace-state-race.md new file mode 100644 index 0000000000..e721c33043 --- /dev/null +++ b/.changeset/fix-workspace-state-race.md @@ -0,0 +1,6 @@ +--- +"@pnpm/workspace.state": patch +"pnpm": patch +--- + +Avoid crashing when the workspace state cache is partially written or malformed. diff --git a/pacquet/crates/workspace-state/Cargo.toml b/pacquet/crates/workspace-state/Cargo.toml index e821e15963..f45369ad3c 100644 --- a/pacquet/crates/workspace-state/Cargo.toml +++ b/pacquet/crates/workspace-state/Cargo.toml @@ -17,10 +17,10 @@ derive_more = { workspace = true } indexmap = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +tempfile = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } -tempfile = { workspace = true } [lints] workspace = true diff --git a/pacquet/crates/workspace-state/src/lib.rs b/pacquet/crates/workspace-state/src/lib.rs index f84163fd7d..79892f9d5b 100644 --- a/pacquet/crates/workspace-state/src/lib.rs +++ b/pacquet/crates/workspace-state/src/lib.rs @@ -17,10 +17,12 @@ use pacquet_diagnostics::miette::{self, Diagnostic}; use serde::{Deserialize, Serialize}; use std::{ collections::BTreeMap, - fs, io, + fs, + io::{self, Write}, path::{Path, PathBuf}, time::{SystemTime, UNIX_EPOCH}, }; +use tempfile::NamedTempFile; /// Basename of the workspace-state file, written inside `node_modules/`. /// @@ -177,10 +179,17 @@ pub enum UpdateWorkspaceStateError { /// Write `state` to `/node_modules/.pnpm-workspace-state-v1.json`. /// -/// Mirrors upstream's [`updateWorkspaceState`](https://github.com/pnpm/pnpm/blob/7ff112bac6/workspace/state/src/updateWorkspaceState.ts): -/// `JSON.stringify(state, undefined, 2) + '\n'`. `serde_json`'s pretty -/// printer uses the same 2-space indent and `": "` separator as JS, so -/// the on-disk bytes round-trip cleanly between the two writers. +/// Writes to a temporary file in the same directory, then atomically +/// renames it into place, so a concurrent reader — pnpm or pacquet — +/// never observes a half-written file. Mirrors upstream's +/// [`updateWorkspaceState`](https://github.com/pnpm/pnpm/blob/7ff112bac6/workspace/state/src/updateWorkspaceState.ts), +/// which writes through `write-file-atomic` for the same reason +/// ([#12020](https://github.com/pnpm/pnpm/issues/12020)). +/// +/// The serialized bytes are `JSON.stringify(state, undefined, 2) + '\n'`: +/// `serde_json`'s pretty printer uses the same 2-space indent and `": "` +/// separator as JS, so the on-disk bytes round-trip cleanly between the +/// two writers. pub fn update_workspace_state( workspace_dir: &Path, state: &WorkspaceState, @@ -194,8 +203,17 @@ pub fn update_workspace_state( let mut serialized = serde_json::to_string_pretty(state).map_err(UpdateWorkspaceStateError::SerializeJson)?; serialized.push('\n'); - fs::write(&file_path, serialized.as_bytes()) - .map_err(|source| UpdateWorkspaceStateError::WriteFile { path: file_path, source }) + let mut temp = NamedTempFile::new_in(parent).map_err(|source| { + UpdateWorkspaceStateError::WriteFile { path: file_path.clone(), source } + })?; + temp.write_all(serialized.as_bytes()).map_err(|source| { + UpdateWorkspaceStateError::WriteFile { path: file_path.clone(), source } + })?; + temp.persist(&file_path).map_err(|error| UpdateWorkspaceStateError::WriteFile { + path: file_path, + source: error.error, + })?; + Ok(()) } /// Read the workspace state file at `/node_modules/.pnpm-workspace-state-v1.json`. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6839614edc..b46a022ca2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -9957,6 +9957,9 @@ importers: ramda: specifier: 'catalog:' version: '@pnpm/ramda@0.28.1' + write-file-atomic: + specifier: 'catalog:' + version: 7.0.1 devDependencies: '@jest/globals': specifier: 'catalog:' @@ -9973,6 +9976,9 @@ importers: '@types/ramda': specifier: 'catalog:' version: 0.31.1 + '@types/write-file-atomic': + specifier: 'catalog:' + version: 4.0.3 workspace/workspace-manifest-reader: dependencies: diff --git a/workspace/state/package.json b/workspace/state/package.json index 5602fc14b4..9d06d3acec 100644 --- a/workspace/state/package.json +++ b/workspace/state/package.json @@ -35,7 +35,8 @@ "@pnpm/catalogs.types": "workspace:*", "@pnpm/config.reader": "workspace:*", "@pnpm/types": "workspace:*", - "ramda": "catalog:" + "ramda": "catalog:", + "write-file-atomic": "catalog:" }, "peerDependencies": { "@pnpm/logger": "catalog:" @@ -45,7 +46,8 @@ "@pnpm/logger": "workspace:*", "@pnpm/prepare": "workspace:*", "@pnpm/workspace.state": "workspace:*", - "@types/ramda": "catalog:" + "@types/ramda": "catalog:", + "@types/write-file-atomic": "catalog:" }, "engines": { "node": ">=22.13" diff --git a/workspace/state/src/loadWorkspaceState.ts b/workspace/state/src/loadWorkspaceState.ts index 02176cdde2..98908e3dc5 100644 --- a/workspace/state/src/loadWorkspaceState.ts +++ b/workspace/state/src/loadWorkspaceState.ts @@ -18,5 +18,12 @@ export function loadWorkspaceState (workspaceDir: string): WorkspaceState | unde } throw error } - return JSON.parse(cacheFileContent) as WorkspaceState + try { + return JSON.parse(cacheFileContent) as WorkspaceState + } catch (error) { + if (util.types.isNativeError(error) && error.name === 'SyntaxError') { + return undefined + } + throw error + } } diff --git a/workspace/state/src/updateWorkspaceState.ts b/workspace/state/src/updateWorkspaceState.ts index a1aa8a71b0..164aab4751 100644 --- a/workspace/state/src/updateWorkspaceState.ts +++ b/workspace/state/src/updateWorkspaceState.ts @@ -3,6 +3,7 @@ import path from 'node:path' import { logger } from '@pnpm/logger' import type { ConfigDependencies } from '@pnpm/types' +import writeFileAtomic from 'write-file-atomic' import { createWorkspaceState } from './createWorkspaceState.js' import { getFilePath } from './filePath.js' @@ -23,5 +24,5 @@ export async function updateWorkspaceState (opts: UpdateWorkspaceStateOptions): const workspaceStateJSON = JSON.stringify(workspaceState, undefined, 2) + '\n' const cacheFile = getFilePath(opts.workspaceDir) await fs.promises.mkdir(path.dirname(cacheFile), { recursive: true }) - await fs.promises.writeFile(cacheFile, workspaceStateJSON) + await writeFileAtomic(cacheFile, workspaceStateJSON) } diff --git a/workspace/state/test/loadWorkspaceState.test.ts b/workspace/state/test/loadWorkspaceState.test.ts index c1db7e850b..51ad4c3057 100644 --- a/workspace/state/test/loadWorkspaceState.test.ts +++ b/workspace/state/test/loadWorkspaceState.test.ts @@ -71,3 +71,15 @@ test('loadWorkspaceState() when cache file exists and is correct', async () => { expect(loadWorkspaceState(workspaceDir)).toStrictEqual(workspaceState) expect(jest.mocked(logger.debug).mock.calls).toStrictEqual(expectedLoggerCalls) }) + +test('loadWorkspaceState() when cache file contains partial JSON', async () => { + prepareEmpty() + + const workspaceDir = process.cwd() + const cacheFile = getFilePath(workspaceDir) + fs.mkdirSync(path.dirname(cacheFile), { recursive: true }) + fs.writeFileSync(cacheFile, '{\n "settings": ') + + expect(loadWorkspaceState(workspaceDir)).toBeUndefined() + expect(jest.mocked(logger.debug).mock.calls).toStrictEqual(expectedLoggerCalls) +})