feat(find-packages): verify format of workspace manifest file (#7273) (#7278)

close #7273

---------

Co-authored-by: Itamar Zwi <itamarz@amplicy.io>
Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/fifty-pans-think.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/workspace.read-manifest": major
+---
+
+Initial release.

.changeset/witty-scissors-smell.md

@@ -0,0 +1,5 @@
+---
+"pnpm": patch
+---
+
+Throw an error on invalid `pnpm-workspace.yaml` file [#7273](https://github.com/pnpm/pnpm/issues/7273).

pnpm-lock.yaml

@@ -6168,9 +6168,6 @@ importers:
       '@pnpm/cli-utils':
         specifier: workspace:*
         version: link:../../cli/cli-utils
-      '@pnpm/constants':
-        specifier: workspace:*
-        version: link:../../packages/constants
       '@pnpm/fs.find-packages':
         specifier: workspace:*
         version: link:../../fs/find-packages
@@ -6183,9 +6180,9 @@ importers:
       '@pnpm/util.lex-comparator':
         specifier: 1.0.0
         version: 1.0.0
-      read-yaml-file:
-        specifier: ^2.1.0
-        version: 2.1.0
+      '@pnpm/workspace.read-manifest':
+        specifier: workspace:*
+        version: link:../read-manifest
     devDependencies:
       '@pnpm/workspace.find-packages':
         specifier: workspace:*
@@ -6229,6 +6226,22 @@ importers:
         specifier: 1.0.0
         version: 1.0.0
 
+  workspace/read-manifest:
+    dependencies:
+      '@pnpm/constants':
+        specifier: workspace:*
+        version: link:../../packages/constants
+      '@pnpm/error':
+        specifier: workspace:*
+        version: link:../../packages/error
+      read-yaml-file:
+        specifier: ^2.1.0
+        version: 2.1.0
+    devDependencies:
+      '@pnpm/workspace.read-manifest':
+        specifier: workspace:*
+        version: 'link:'
+
   workspace/resolve-workspace-range:
     dependencies:
       semver:
@@ -8991,7 +9004,7 @@ packages:
   /@types/byline@4.2.35:
     resolution: {integrity: sha512-YRfEGhvLQrA1/ixrJ95/vqin4J+hc0OIHH89rWGE2q0Tn9Jy6BhPZTiCLV1X39VJMOoq8UCUUFN+WD+gnmBjhw==}
     dependencies:
-      '@types/node': 20.8.8
+      '@types/node': 16.18.59
     dev: true
 
   /@types/cacache@17.0.1:
@@ -9005,7 +9018,7 @@ packages:
     dependencies:
       '@types/http-cache-semantics': 4.0.3
       '@types/keyv': 3.1.4
-      '@types/node': 20.8.8
+      '@types/node': 16.18.59
       '@types/responselike': 1.0.2
 
   /@types/cross-spawn@6.0.4:
@@ -9111,7 +9124,7 @@ packages:
   /@types/keyv@3.1.4:
     resolution: {integrity: sha512-BQ5aZNSCpj7D6K2ksrRCTmKRLEpnPvWDiLPfoGyhZ++8YtiK9d/3DBKPJgry359X/P1PfruyYwvnvwFjuEiEIg==}
     dependencies:
-      '@types/node': 20.8.8
+      '@types/node': 16.18.59
 
   /@types/lodash.clonedeep@4.5.8:
     resolution: {integrity: sha512-I5toZLLfTvhnuAnejjVgSpBSLSC316bVURbI0sCYI0dKY3jaJgOg2arfPC6miTNnHRi/Tk/J6BB+kzT3iB5mcw==}
@@ -9267,7 +9280,7 @@ packages:
   /@types/responselike@1.0.2:
     resolution: {integrity: sha512-/4YQT5Kp6HxUDb4yhRkm0bJ7TbjvTddqX7PZ5hz6qV3pxSo72f/6YPRo+Mu2DU307tm9IioO69l7uAwn5XNcFA==}
     dependencies:
-      '@types/node': 20.8.8
+      '@types/node': 16.18.59
 
   /@types/retry@0.12.4:
     resolution: {integrity: sha512-l1YzFLj8Y6OhLdt7HKXlz56DoEmksB7qR8KVk+MpFsS4duwnoszLgDlLxJB0vgSqtg/rAS5gmYg5Bjw2sMJ8Ew==}
@@ -9664,7 +9677,7 @@ packages:
     resolution: {integrity: sha512-YmG+oTBCyrAoMIx5g2I9CfyurSpHyoan+9SCj7laaFKseOe3lFEyIVKvwRBQMmSt8uzh+eY5RWeQnoyyOs6AbA==}
     engines: {node: '>=14.15.0'}
     peerDependencies:
-      '@yarnpkg/fslib': 3.0.0-rc.25
+      '@yarnpkg/fslib': ^3.0.0-rc.25
     dependencies:
       '@types/emscripten': 1.39.9
       '@yarnpkg/fslib': 3.0.0-rc.25

workspace/find-packages/package.json

@@ -30,11 +30,10 @@
   "homepage": "https://github.com/pnpm/pnpm/blob/main/workspace/find-packages#readme",
   "dependencies": {
     "@pnpm/cli-utils": "workspace:*",
-    "@pnpm/constants": "workspace:*",
     "@pnpm/fs.find-packages": "workspace:*",
     "@pnpm/types": "workspace:*",
     "@pnpm/util.lex-comparator": "1.0.0",
-    "read-yaml-file": "^2.1.0"
+    "@pnpm/workspace.read-manifest": "workspace:*"
   },
   "funding": "https://opencollective.com/pnpm",
   "devDependencies": {

workspace/find-packages/src/index.ts

@@ -1,11 +1,9 @@
-import path from 'path'
 import { packageIsInstallable } from '@pnpm/cli-utils'
-import { WORKSPACE_MANIFEST_FILENAME } from '@pnpm/constants'
 import { type ProjectManifest, type Project, type SupportedArchitectures } from '@pnpm/types'
+import { readWorkspaceManifest } from '@pnpm/workspace.read-manifest'
 import { lexCompare } from '@pnpm/util.lex-comparator'
 import { findPackages } from '@pnpm/fs.find-packages'
 import { logger } from '@pnpm/logger'
-import readYamlFile from 'read-yaml-file'
 
 export type { Project }
 
@@ -40,8 +38,8 @@ export async function findWorkspacePackages (
 export async function findWorkspacePackagesNoCheck (workspaceRoot: string, opts?: { patterns?: string[] }): Promise<Project[]> {
   let patterns = opts?.patterns
   if (patterns == null) {
-    const packagesManifest = await requirePackagesManifest(workspaceRoot)
-    patterns = packagesManifest?.packages ?? undefined
+    const workspaceManifest = await readWorkspaceManifest(workspaceRoot)
+    patterns = workspaceManifest?.packages
   }
   const pkgs = await findPackages(workspaceRoot, {
     ignore: [
@@ -55,17 +53,6 @@ export async function findWorkspacePackagesNoCheck (workspaceRoot: string, opts?
   return pkgs
 }
 
-async function requirePackagesManifest (dir: string): Promise<{ packages?: string[] } | null> {
-  try {
-    return await readYamlFile<{ packages?: string[] }>(path.join(dir, WORKSPACE_MANIFEST_FILENAME))
-  } catch (err: any) { // eslint-disable-line
-    if (err['code'] === 'ENOENT') {
-      return null
-    }
-    throw err
-  }
-}
-
 type ArrayOfWorkspacePackagesToMapResult = Record<string, Record<string, Pick<Project, 'manifest'>>>
 
 export function arrayOfWorkspacePackagesToMap (

workspace/find-packages/tsconfig.json

@@ -16,10 +16,10 @@
       "path": "../../fs/find-packages"
     },
     {
-      "path": "../../packages/constants"
+      "path": "../../packages/types"
     },
     {
-      "path": "../../packages/types"
+      "path": "../read-manifest"
     }
   ],
   "composite": true

workspace/read-manifest/README.md

@@ -0,0 +1,13 @@
+# @pnpm/workspace.read-manifest
+
+> Reads a workspace manifest file
+
+## Install
+
+```
+pnpm add @pnpm/workspace.read-manifest
+```
+
+## LICENSE
+
+MIT

workspace/read-manifest/jest.config.js

@@ -0,0 +1 @@
+module.exports = require('../../jest.config.js')

workspace/read-manifest/package.json

@@ -0,0 +1,43 @@
+{
+  "name": "@pnpm/workspace.read-manifest",
+  "version": "0.0.0",
+  "description": "Reads a workspace manifest file",
+  "main": "lib/index.js",
+  "types": "lib/index.d.ts",
+  "files": [
+    "lib",
+    "!*.map"
+  ],
+  "engines": {
+    "node": ">=16.14"
+  },
+  "scripts": {
+    "lint": "eslint \"src/**/*.ts\" \"test/**/*.ts\"",
+    "_test": "jest",
+    "test": "pnpm run compile && pnpm run _test",
+    "prepublishOnly": "pnpm run compile",
+    "compile": "tsc --build && pnpm run lint --fix"
+  },
+  "repository": "https://github.com/pnpm/pnpm/blob/main/workspace/read-manifest",
+  "keywords": [
+    "pnpm8",
+    "pnpm"
+  ],
+  "license": "MIT",
+  "bugs": {
+    "url": "https://github.com/pnpm/pnpm/issues"
+  },
+  "homepage": "https://github.com/pnpm/pnpm/blob/main/workspace/read-manifest#readme",
+  "dependencies": {
+    "@pnpm/error": "workspace:*",
+    "@pnpm/constants": "workspace:*",
+    "read-yaml-file": "^2.1.0"
+  },
+  "funding": "https://opencollective.com/pnpm",
+  "devDependencies": {
+    "@pnpm/workspace.read-manifest": "workspace:*"
+  },
+  "exports": {
+    ".": "./lib/index.js"
+  }
+}

workspace/read-manifest/src/index.ts

@@ -0,0 +1,83 @@
+import { WORKSPACE_MANIFEST_FILENAME } from '@pnpm/constants'
+import { PnpmError } from '@pnpm/error'
+import path from 'node:path'
+import readYamlFile from 'read-yaml-file'
+
+export interface WorkspaceManifest {
+  packages?: string[]
+}
+
+export async function readWorkspaceManifest (dir: string): Promise<WorkspaceManifest | undefined> {
+  const manifest = await readManifestRaw(dir)
+  if (validateWorkspaceManifest(manifest)) {
+    return manifest
+  }
+
+  return undefined
+}
+
+async function readManifestRaw (dir: string): Promise<unknown> {
+  try {
+    return await readYamlFile<WorkspaceManifest>(path.join(dir, WORKSPACE_MANIFEST_FILENAME))
+  } catch (err: any) { // eslint-disable-line
+    // File not exists is the same as empty file (undefined)
+    if (err['code'] === 'ENOENT') {
+      return undefined
+    }
+
+    // Any other error (missing perm, invalid yaml, etc.) fails the process
+    throw err
+  }
+}
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+function validateWorkspaceManifest (manifest: any): manifest is WorkspaceManifest | undefined {
+  if (manifest === undefined) {
+    // Empty manifest is ok
+    return true
+  }
+
+  if (manifest === null) {
+    throw new InvalidWorkspaceManifestError('Expected object but found - null')
+  }
+
+  if (typeof manifest !== 'object') {
+    throw new InvalidWorkspaceManifestError(`Expected object but found - ${typeof manifest}`)
+  }
+
+  if (Array.isArray(manifest)) {
+    throw new InvalidWorkspaceManifestError('Expected object but found - array')
+  }
+
+  if (Object.keys(manifest).length === 0) {
+    // manifest content `{}` is ok
+    return true
+  }
+
+  if (!manifest.packages) {
+    throw new InvalidWorkspaceManifestError('packages field missing or empty')
+  }
+
+  if (!Array.isArray(manifest.packages)) {
+    throw new InvalidWorkspaceManifestError('packages field is not an array')
+  }
+
+  for (const pkg of manifest.packages) {
+    if (!pkg) {
+      throw new InvalidWorkspaceManifestError('Missing or empty package')
+    }
+
+    const type = typeof pkg
+    if (type !== 'string') {
+      throw new InvalidWorkspaceManifestError(`Invalid package type - ${type}`)
+    }
+  }
+
+  return true
+}
+
+class InvalidWorkspaceManifestError extends PnpmError {
+  constructor (message: string) {
+    super('INVALID_WORKSPACE_CONFIGURATION', message)
+  }
+}

workspace/read-manifest/test/__fixtures__/array/pnpm-workspace.yaml

@@ -0,0 +1 @@
+- "pkg"

workspace/read-manifest/test/__fixtures__/no-workspace-file/pkg/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "pkg",
+  "version": "1.0.0"
+}

workspace/read-manifest/test/__fixtures__/null/pnpm-workspace.yaml

@@ -0,0 +1 @@
+null

workspace/read-manifest/test/__fixtures__/ok/pnpm-workspace.yaml

@@ -0,0 +1,3 @@
+packages:
+  - "packages/**"
+  - "types"

workspace/read-manifest/test/__fixtures__/packages-contains-empty/pnpm-workspace.yaml

@@ -0,0 +1,3 @@
+packages:
+  - "pkg"
+  -

workspace/read-manifest/test/__fixtures__/packages-contains-number/pnpm-workspace.yaml

@@ -0,0 +1,2 @@
+packages:
+  - 1

workspace/read-manifest/test/__fixtures__/packages-empty/pnpm-workspace.yaml

@@ -0,0 +1 @@
+packages:

workspace/read-manifest/test/__fixtures__/packages-string/pnpm-workspace.yaml

@@ -0,0 +1 @@
+packages: "pkg"

workspace/read-manifest/test/__fixtures__/string/pnpm-workspace.yaml

@@ -0,0 +1 @@
+"pkg"

workspace/read-manifest/test/index.ts

@@ -0,0 +1,52 @@
+import { readWorkspaceManifest } from '@pnpm/workspace.read-manifest'
+import path from 'node:path'
+
+test('readWorkspaceManifest() works with a valid workspace file', async () => {
+  const manifest = await readWorkspaceManifest(path.join(__dirname, '__fixtures__/ok'))
+
+  expect(manifest).toEqual({
+    packages: ['packages/**', 'types'],
+  })
+})
+
+test('readWorkspaceManifest() throws on string content', async () => {
+  await expect(
+    readWorkspaceManifest(path.join(__dirname, '__fixtures__/string'))
+  ).rejects.toThrow('Expected object but found - string')
+})
+
+test('readWorkspaceManifest() throws on array content', async () => {
+  await expect(
+    readWorkspaceManifest(path.join(__dirname, '__fixtures__/array'))
+  ).rejects.toThrow('Expected object but found - array')
+})
+
+test('readWorkspaceManifest() throws on empty packages field', async () => {
+  await expect(
+    readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-empty'))
+  ).rejects.toThrow('packages field missing or empty')
+})
+
+test('readWorkspaceManifest() throws on string packages field', async () => {
+  await expect(
+    readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-string'))
+  ).rejects.toThrow('packages field is not an array')
+})
+
+test('readWorkspaceManifest() throws on empty package', async () => {
+  await expect(
+    readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-contains-empty'))
+  ).rejects.toThrow('Missing or empty package')
+})
+
+test('readWorkspaceManifest() throws on numeric package', async () => {
+  await expect(
+    readWorkspaceManifest(path.join(__dirname, '__fixtures__/packages-contains-number'))
+  ).rejects.toThrow('Invalid package type - number')
+})
+
+test('readWorkspaceManifest() works when no workspace file is present', async () => {
+  const manifest = await readWorkspaceManifest(path.join(__dirname, '__fixtures__/no-workspace-file'))
+
+  expect(manifest).toBeUndefined()
+})

workspace/read-manifest/tsconfig.json

@@ -0,0 +1,20 @@
+{
+  "extends": "@pnpm/tsconfig",
+  "compilerOptions": {
+    "outDir": "lib",
+    "rootDir": "src"
+  },
+  "include": [
+    "src/**/*.ts",
+    "../../__typings__/**/*.d.ts"
+  ],
+  "references": [
+    {
+      "path": "../../packages/constants"
+    },
+    {
+      "path": "../../packages/error"
+    }
+  ],
+  "composite": true
+}

workspace/read-manifest/tsconfig.lint.json

@@ -0,0 +1,8 @@
+{
+  "extends": "./tsconfig.json",
+  "include": [
+    "src/**/*.ts",
+    "test/**/*.ts",
+    "../../__typings__/**/*.d.ts"
+  ]
+}