fix(lockfile): hide content from parse errors (#13101)

Broken lockfile errors rendered parser-provided source snippets. A repository can commit pnpm-lock.yaml as a symlink, so the snippet could quote content from a readable file outside the repository.

Format parse failures from their structured reason and location instead. Pacquet also stops exposing the snippet-bearing parser error through its diagnostic source chain.

Closes pnpm/pnpm#13094

.changeset/safe-broken-lockfile-errors.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/lockfile.fs": patch
+"pacquet": patch
+"pnpm": patch
+---
+
+Prevent broken-lockfile errors from including snippets of the lockfile's contents.

pnpm/crates/lockfile/src/env_lockfile.rs

@@ -116,8 +116,8 @@ impl EnvLockfile {
         let Some(env_doc) = extract_env_document(&content) else {
             return Ok(None);
         };
-        let mut env: EnvLockfile =
-            serde_saphyr::from_str(env_doc).map_err(LoadLockfileError::ParseYaml)?;
+        let mut env: EnvLockfile = serde_saphyr::from_str(env_doc)
+            .map_err(|source| LoadLockfileError::parse_yaml(&path, &source))?;
         env.root_importer_mut();
         Ok(Some(env))
     }

pnpm/crates/lockfile/src/load_lockfile.rs

@@ -2,10 +2,11 @@ use crate::{Lockfile, extract_main_document};
 use derive_more::{Display, Error};
 use pacquet_diagnostics::miette::{self, Diagnostic};
 use pipe_trait::Pipe;
+use serde_saphyr::MessageFormatter;
 use std::{
     env, fs,
     io::{self, ErrorKind},
-    path::Path,
+    path::{Path, PathBuf},
 };
 
 /// Error when reading lockfile the filesystem.
@@ -20,9 +21,29 @@ pub enum LoadLockfileError {
     #[diagnostic(code(ERR_PNPM_LOCKFILE_READ_FILE))]
     ReadFile(io::Error),
 
-    #[display("Failed to parse lockfile content as YAML: {_0}")]
-    #[diagnostic(code(ERR_PNPM_LOCKFILE_PARSE_YAML))]
-    ParseYaml(serde_saphyr::Error),
+    #[display(
+        "The lockfile at \"{}\" is broken: {}",
+        path.display(),
+        reason
+    )]
+    #[diagnostic(code(ERR_PNPM_BROKEN_LOCKFILE))]
+    ParseYaml { path: PathBuf, reason: String },
+}
+
+impl LoadLockfileError {
+    pub(super) fn parse_yaml(path: &Path, source: &serde_saphyr::Error) -> Self {
+        Self::ParseYaml { path: path.to_path_buf(), reason: format_yaml_error(source) }
+    }
+}
+
+fn format_yaml_error(error: &serde_saphyr::Error) -> String {
+    let error = error.without_snippet();
+    let reason = serde_saphyr::DefaultMessageFormatter.format_message(error);
+    if let Some(location) = error.location() {
+        format!("{reason} ({}:{})", location.line(), location.column())
+    } else {
+        reason.into_owned()
+    }
 }
 
 impl Lockfile {
@@ -96,7 +117,7 @@ impl Lockfile {
                 lockfile.reconstruct_missing_directory_resolutions();
                 Some(lockfile)
             })
-            .map_err(LoadLockfileError::ParseYaml)
+            .map_err(|source| LoadLockfileError::parse_yaml(file_path, &source))
     }
 }
 

pnpm/crates/lockfile/src/load_lockfile/tests.rs

@@ -2,6 +2,7 @@ use crate::{
     DirectoryResolution, ImporterDepVersion, Lockfile, LockfileResolution, PackageKey, PkgName,
     SnapshotDepRef,
 };
+use pacquet_diagnostics::miette::Diagnostic;
 use pretty_assertions::assert_eq;
 use tempfile::tempdir;
 use text_block_macros::text_block;
@@ -96,6 +97,34 @@ fn env_only_lockfile_loads_as_none() {
     assert!(result.is_none(), "expected None for env-only lockfile, got: {result:?}");
 }
 
+#[test]
+fn parse_error_does_not_include_lockfile_content() {
+    let dir = tempdir().expect("create tempdir");
+    let secret = "aws_secret_access_key = marker-secret";
+    std::fs::write(dir.path().join(Lockfile::FILE_NAME), format!("[default]\n{secret}\n"))
+        .expect("write broken lockfile");
+
+    let error = Lockfile::load_wanted_from_dir(dir.path()).expect_err("lockfile must be broken");
+    let message = error.to_string();
+
+    assert_eq!(error.code().expect("diagnostic code").to_string(), "ERR_PNPM_BROKEN_LOCKFILE");
+    assert!(
+        message.starts_with(&format!(
+            r#"The lockfile at "{}" is broken: "#,
+            dir.path().join(Lockfile::FILE_NAME).display()
+        )),
+        "unexpected error: {message}",
+    );
+    assert!(message.contains("(1:1)"), "unexpected error: {message}");
+    assert!(!message.contains(secret), "error included lockfile content: {message}");
+    assert!(
+        std::error::Error::source(&error).is_none(),
+        "parse error source could expose lockfile content",
+    );
+    let report = format!("{:?}", pacquet_diagnostics::miette::Report::new(error));
+    assert!(!report.contains(secret), "diagnostic included lockfile content: {report}");
+}
+
 /// Heuristic-boundary check: a dropped directory resolution is
 /// reconstructed for a pruned `file:` peer-variant, but never for a
 /// `file:` tarball.

pnpm11/lockfile/fs/src/read.ts

@@ -141,7 +141,7 @@ async function _read (
     hadConflicts = false
   } catch (err: unknown) {
     if (!opts.autofixMergeConflicts || !isDiff(lockfileRawContent)) {
-      throw new PnpmError('BROKEN_LOCKFILE', `The lockfile at "${lockfilePath}" is broken: ${(err as Error).message}`)
+      throw new PnpmError('BROKEN_LOCKFILE', `The lockfile at "${lockfilePath}" is broken: ${formatLockfileError(err)}`)
     }
     hadConflicts = true
     lockfile = autofixMergeConflicts(lockfileRawContent)
@@ -182,6 +182,34 @@ async function _read (
   throw new LockfileBreakingChangeError(lockfilePath)
 }
 
+function formatLockfileError (err: unknown): string {
+  if (isYamlException(err)) {
+    const reason = typeof err.reason === 'string' ? err.reason : 'Unable to parse YAML'
+    const line = err.mark?.line
+    const column = err.mark?.column
+    const position = typeof line === 'number' && Number.isFinite(line) &&
+      typeof column === 'number' && Number.isFinite(column)
+      ? ` (${line + 1}:${column + 1})`
+      : ''
+    return `${reason}${position}`
+  }
+  return util.types.isNativeError(err) ? err.message : String(err)
+}
+
+function isYamlException (err: unknown): err is YamlExceptionLike {
+  return typeof err === 'object' && err !== null &&
+    'name' in err && err.name === 'YAMLException'
+}
+
+interface YamlExceptionLike {
+  name: 'YAMLException'
+  reason?: unknown
+  mark?: {
+    line?: unknown
+    column?: unknown
+  } | null
+}
+
 export function createLockfileObject (
   importerIds: ProjectId[],
   opts: {

pnpm11/lockfile/fs/test/read.test.ts

@@ -1,7 +1,9 @@
+import { writeFile } from 'node:fs/promises'
 import path from 'node:path'
 
 import { expect, jest, test } from '@jest/globals'
 import type { DepPath, ProjectId } from '@pnpm/types'
+import yaml from 'js-yaml'
 import { temporaryDirectory } from 'tempy'
 
 jest.unstable_mockModule('@pnpm/network.git-utils', () => ({ getCurrentBranch: jest.fn() }))
@@ -48,6 +50,39 @@ test('readWantedLockfile()', async () => {
   ).rejects.toMatchObject({ code: 'ERR_PNPM_LOCKFILE_BREAKING_CHANGE' })
 })
 
+test('readWantedLockfile() does not include lockfile content in parse errors', async () => {
+  const projectPath = temporaryDirectory()
+  const secret = 'aws_secret_access_key = marker-secret'
+  await writeFile(path.join(projectPath, 'pnpm-lock.yaml'), `[default]\n${secret}\n`)
+
+  const error = await readWantedLockfile(projectPath, { ignoreIncompatible: false }).catch((err: unknown) => err)
+
+  expect(error).toMatchObject({
+    code: 'ERR_PNPM_BROKEN_LOCKFILE',
+    message: expect.stringContaining(`The lockfile at "${path.join(projectPath, 'pnpm-lock.yaml')}" is broken:`),
+  })
+  expect(error).toMatchObject({ message: expect.stringContaining('(2:1)') })
+  expect(error).toMatchObject({ message: expect.not.stringContaining(secret) })
+})
+
+test('readWantedLockfile() does not use a YAML exception message when its reason is missing', async () => {
+  const projectPath = temporaryDirectory()
+  const secret = 'aws_secret_access_key = marker-secret'
+  await writeFile(path.join(projectPath, 'pnpm-lock.yaml'), 'broken')
+  jest.spyOn(yaml, 'load').mockImplementationOnce(() => {
+    throw {
+      name: 'YAMLException',
+      message: `Unable to parse YAML\n${secret}`,
+      mark: { line: 1, column: 2 },
+    }
+  })
+
+  await expect(readWantedLockfile(projectPath, { ignoreIncompatible: false })).rejects.toMatchObject({
+    code: 'ERR_PNPM_BROKEN_LOCKFILE',
+    message: `The lockfile at "${path.join(projectPath, 'pnpm-lock.yaml')}" is broken: Unable to parse YAML (2:3)`,
+  })
+})
+
 test('readWantedLockfile() when lockfileVersion is a string', async () => {
   {
     const lockfile = await readWantedLockfile(path.join('fixtures', '4'), {