perf: lazily load README when embedding publish manifest (#12278)

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/lazy-readme-embed.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/releasing.exportable-manifest": patch
+"@pnpm/releasing.commands": patch
+"pnpm": patch
+---
+
+Avoid reading `README.md` from disk when publishing if the publish manifest already provides a `readme` field. The README is now only read lazily, inside `createExportableManifest`, when it is actually needed.

releasing/commands/src/publish/pack.ts

@@ -352,14 +352,6 @@ function preventBundledDependenciesWithoutHoistedNodeLinker (nodeLinker: Config[
   }
 }
 
-async function readReadmeFile (projectDir: string): Promise<string | undefined> {
-  const files = await fs.promises.readdir(projectDir)
-  const readmePath = files.find(name => /readme\.md$/i.test(name))
-  const readmeFile = readmePath ? await fs.promises.readFile(path.join(projectDir, readmePath), 'utf8') : undefined
-
-  return readmeFile
-}
-
 async function packPkg (opts: {
   destFile: string
   filesMap: Record<string, string>
@@ -405,11 +397,10 @@ async function createPublishManifest (opts: {
   skipManifestObfuscation?: boolean
 }): Promise<ExportedManifest> {
   const { projectDir, embedReadme, modulesDir, manifest, catalogs, hooks, skipManifestObfuscation } = opts
-  const readmeFile = embedReadme ? await readReadmeFile(projectDir) : undefined
   return createExportableManifest(projectDir, manifest, {
     catalogs,
     hooks,
-    readmeFile,
+    embedReadme,
     modulesDir,
     skipManifestObfuscation,
   })

releasing/exportable-manifest/src/index.ts

@@ -1,3 +1,4 @@
+import fs from 'node:fs'
 import path from 'node:path'
 
 import { type CatalogResolver, resolveFromCatalog } from '@pnpm/catalogs.resolver'
@@ -29,7 +30,16 @@ export interface MakePublishManifestOptions {
   hooks?: Hooks
   modulesDir?: string
   skipManifestObfuscation?: boolean
-  readmeFile?: string
+  embedReadme?: boolean
+}
+
+async function readReadmeFile (projectDir: string): Promise<string | undefined> {
+  const entries = await fs.promises.readdir(projectDir, { withFileTypes: true })
+  // Only embed a regular README.md file. A symlink could point outside the
+  // project and leak its target's contents into the published manifest.
+  const readmeEntry = entries.find((entry) => entry.isFile() && /^readme\.md$/i.test(entry.name))
+  if (readmeEntry == null) return undefined
+  return fs.promises.readFile(path.join(projectDir, readmeEntry.name), 'utf8')
 }
 
 export async function createExportableManifest (
@@ -72,8 +82,11 @@ export async function createExportableManifest (
 
   overridePublishConfig(publishManifest)
 
-  if (opts?.readmeFile) {
-    publishManifest.readme ??= opts.readmeFile
+  if (publishManifest.readme == null && opts?.embedReadme) {
+    const readme = await readReadmeFile(dir)
+    if (readme != null) {
+      publishManifest.readme = readme
+    }
   }
 
   for (const hook of opts?.hooks?.beforePacking ?? []) {

releasing/exportable-manifest/test/index.test.ts

@@ -1,4 +1,6 @@
 /// <reference path="../../../__typings__/index.d.ts"/>
+import fs from 'node:fs'
+import os from 'node:os'
 import path from 'node:path'
 
 import { expect, test } from '@jest/globals'
@@ -119,15 +121,17 @@ test('skipManifestObfuscation does not mutate the original manifest', async () =
     },
   }
 
-  expect(await createExportableManifest(process.cwd(), manifest, {
-    ...defaultOpts,
-    skipManifestObfuscation: true,
-    readmeFile: 'readme content',
-  })).toStrictEqual({
-    name: 'foo',
-    version: '1.0.0',
-    main: './dist/index.js',
-    readme: 'readme content',
+  await withTempProjectReadme('readme content', async (projectDir) => {
+    expect(await createExportableManifest(projectDir, manifest, {
+      ...defaultOpts,
+      skipManifestObfuscation: true,
+      embedReadme: true,
+    })).toStrictEqual({
+      name: 'foo',
+      version: '1.0.0',
+      main: './dist/index.js',
+      readme: 'readme content',
+    })
   })
 
   expect(manifest).toStrictEqual({
@@ -140,16 +144,55 @@ test('skipManifestObfuscation does not mutate the original manifest', async () =
 })
 
 test('readme added to published manifest', async () => {
-  expect(await createExportableManifest(process.cwd(), {
-    name: 'foo',
-    version: '1.0.0',
-  }, { ...defaultOpts, readmeFile: 'readme content' })).toStrictEqual({
-    name: 'foo',
-    version: '1.0.0',
-    readme: 'readme content',
+  await withTempProjectReadme('readme content', async (projectDir) => {
+    expect(await createExportableManifest(projectDir, {
+      name: 'foo',
+      version: '1.0.0',
+    }, {
+      ...defaultOpts,
+      embedReadme: true,
+    })).toStrictEqual({
+      name: 'foo',
+      version: '1.0.0',
+      readme: 'readme content',
+    })
   })
 })
 
+;(process.platform === 'win32' ? test.skip : test)('readme is not embedded when README.md is a symlink pointing outside the project', async () => {
+  const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'pnpm-readme-'))
+  try {
+    const secretFile = path.join(tmpDir, 'secret.txt')
+    await fs.promises.writeFile(secretFile, 'secret content', 'utf8')
+    const projectDir = path.join(tmpDir, 'project')
+    await fs.promises.mkdir(projectDir)
+    await fs.promises.symlink(secretFile, path.join(projectDir, 'README.md'))
+
+    expect(await createExportableManifest(projectDir, {
+      name: 'foo',
+      version: '1.0.0',
+    }, {
+      ...defaultOpts,
+      embedReadme: true,
+    })).toStrictEqual({
+      name: 'foo',
+      version: '1.0.0',
+    })
+  } finally {
+    await fs.promises.rm(tmpDir, { recursive: true, force: true })
+  }
+})
+
+async function withTempProjectReadme<T> (readmeContent: string, fn: (projectDir: string) => Promise<T>): Promise<T> {
+  const projectDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'pnpm-readme-'))
+  try {
+    await fs.promises.writeFile(path.join(projectDir, 'README.md'), readmeContent, 'utf8')
+    return await fn(projectDir)
+  } finally {
+    await fs.promises.rm(projectDir, { recursive: true, force: true })
+  }
+}
+
 test('workspace deps are replaced', async () => {
   const manifest: ProjectManifest = {
     name: 'workspace-protocol-package',