From b2d9d78c27e1db07773945315819d9518684d437 Mon Sep 17 00:00:00 2001 From: Gabriel Dugny Date: Thu, 29 Jan 2026 17:43:29 +0100 Subject: [PATCH] fix: defensive package.json reading Signed-off-by: Gabriel Dugny --- packages/cli-common/report.api.md | 2 +- packages/cli-common/src/index.ts | 3 +- packages/cli-common/src/paths.test.ts | 6 +- packages/cli-common/src/paths.ts | 20 +----- packages/cli-common/src/workspaces.test.ts | 69 +++++++++++++++++++ packages/cli-common/src/workspaces.ts | 63 +++++++++++++++++ .../src/pacman/PackageManager.test.ts | 4 +- 7 files changed, 142 insertions(+), 25 deletions(-) create mode 100644 packages/cli-common/src/workspaces.test.ts create mode 100644 packages/cli-common/src/workspaces.ts diff --git a/packages/cli-common/report.api.md b/packages/cli-common/report.api.md index ec146f78c4..324b28880b 100644 --- a/packages/cli-common/report.api.md +++ b/packages/cli-common/report.api.md @@ -24,7 +24,7 @@ export class ExitCodeError extends CustomErrorBase { export function findPaths(searchDir: string): Paths; // @public -export function getWorkspacesPatterns(pkgJson: any): string[]; +export function getWorkspacesPatterns(pkgJson: unknown): string[]; // @public export function isChildPath(base: string, path: string): boolean; diff --git a/packages/cli-common/src/index.ts b/packages/cli-common/src/index.ts index 146af314cd..e142030b88 100644 --- a/packages/cli-common/src/index.ts +++ b/packages/cli-common/src/index.ts @@ -20,7 +20,8 @@ * @packageDocumentation */ -export { findPaths, getWorkspacesPatterns, BACKSTAGE_JSON } from './paths'; +export { findPaths, BACKSTAGE_JSON } from './paths'; +export { getWorkspacesPatterns } from './workspaces'; export { isChildPath } from './isChildPath'; export type { Paths, ResolveFunc } from './paths'; export { bootstrapEnvProxyAgents } from './proxyBootstrap'; diff --git a/packages/cli-common/src/paths.test.ts b/packages/cli-common/src/paths.test.ts index 6ade904e12..f3d7b9b873 100644 --- a/packages/cli-common/src/paths.test.ts +++ b/packages/cli-common/src/paths.test.ts @@ -98,7 +98,9 @@ describe('paths', () => { }); it('findPaths should find workspace root with object', () => { - jest.spyOn(JSON, 'parse').mockReturnValue({ workspaces: { packages: [] } }); + jest + .spyOn(JSON, 'parse') + .mockReturnValue({ workspaces: { packages: ['packages/*'] } }); jest.spyOn(process, 'cwd').mockReturnValue(__dirname); const paths = findPaths(__dirname); @@ -110,7 +112,7 @@ describe('paths', () => { }); it('findPaths should find workspace root with array', () => { - jest.spyOn(JSON, 'parse').mockReturnValue({ workspaces: [] }); + jest.spyOn(JSON, 'parse').mockReturnValue({ workspaces: ['packages/*'] }); jest.spyOn(process, 'cwd').mockReturnValue(__dirname); const paths = findPaths(__dirname); diff --git a/packages/cli-common/src/paths.ts b/packages/cli-common/src/paths.ts index 557af7ccca..efbb05c7bb 100644 --- a/packages/cli-common/src/paths.ts +++ b/packages/cli-common/src/paths.ts @@ -16,6 +16,7 @@ import fs from 'node:fs'; import { dirname, resolve as resolvePath } from 'node:path'; +import { getWorkspacesPatterns } from './workspaces'; /** * A function that takes a set of path fragments and resolves them into a @@ -107,25 +108,6 @@ export function findOwnRootDir(ownDir: string) { return resolvePath(ownDir, '../..'); } -/** - * Gets the workspaces pattern from a package.json object. - * @param pkgJson - The package.json object to get the workspaces pattern from. - * @returns The workspaces patterns (glob not resolved). - * @public - */ -export function getWorkspacesPatterns(pkgJson: any): string[] { - if (Array.isArray(pkgJson.workspaces)) { - return pkgJson.workspaces; - } else if ( - typeof pkgJson.workspaces === 'object' && - pkgJson.workspaces !== null && - Array.isArray(pkgJson.workspaces.packages) - ) { - return pkgJson.workspaces.packages; - } - return []; -} - /** * Find paths related to a package and its execution context. * diff --git a/packages/cli-common/src/workspaces.test.ts b/packages/cli-common/src/workspaces.test.ts new file mode 100644 index 0000000000..7725aa7e6a --- /dev/null +++ b/packages/cli-common/src/workspaces.test.ts @@ -0,0 +1,69 @@ +/* + * Copyright 2026 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { getWorkspacesPatterns } from './workspaces'; + +describe('getWorkspacesPatterns', () => { + it('returns empty array when pkgJson has no workspaces key', () => { + expect(getWorkspacesPatterns({ name: 'pkg' })).toEqual([]); + }); + + it('returns patterns when workspaces is array form', () => { + const patterns = ['packages/*', 'apps/*']; + expect(getWorkspacesPatterns({ workspaces: patterns })).toEqual(patterns); + }); + + it('returns patterns when workspaces is object form with packages', () => { + const patterns = ['packages/*']; + expect( + getWorkspacesPatterns({ workspaces: { packages: patterns } }), + ).toEqual(patterns); + }); + + it('returns empty array when workspaces is object without packages array', () => { + expect(getWorkspacesPatterns({ workspaces: {} })).toEqual([]); + expect(getWorkspacesPatterns({ workspaces: { noMatch: [] } })).toEqual([]); + }); + + it('throws when pkgJson is not an object', () => { + expect(() => getWorkspacesPatterns(null)).toThrow( + 'pkgJson must be an object', + ); + expect(() => getWorkspacesPatterns('string')).toThrow( + 'pkgJson must be an object', + ); + expect(() => getWorkspacesPatterns(42)).toThrow( + 'pkgJson must be an object', + ); + }); + + it('throws when workspaces array contains non-strings', () => { + expect(() => getWorkspacesPatterns({ workspaces: ['a', 1, 'b'] })).toThrow( + 'must be an array of strings', + ); + expect(() => getWorkspacesPatterns({ workspaces: [null] })).toThrow( + 'must be an array of strings', + ); + }); + + it('throws when workspaces.packages contains non-strings', () => { + expect(() => + getWorkspacesPatterns({ + workspaces: { packages: ['a', 1] }, + }), + ).toThrow('must be an array of strings'); + }); +}); diff --git a/packages/cli-common/src/workspaces.ts b/packages/cli-common/src/workspaces.ts new file mode 100644 index 0000000000..17fca511a4 --- /dev/null +++ b/packages/cli-common/src/workspaces.ts @@ -0,0 +1,63 @@ +/* + * Copyright 2026 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +function assertStringArray( + value: unknown, + field: string, +): asserts value is string[] { + if (!Array.isArray(value)) { + throw new Error(`${field} must be an array`); + } + if (!value.every((item): item is string => typeof item === 'string')) { + throw new Error(`${field} must be an array of strings`); + } +} + +/** + * Gets the workspaces patterns from a package.json object. + * + * Supports both the array form (`"workspaces": ["packages/*"]`) and the object + * form (`"workspaces": { "packages": ["packages/*"] }`) from npm/yarn. + * + * @param pkgJson - The package.json object. Type as `unknown` so callers must pass parsed JSON; an object type check is performed. + * @returns The workspaces patterns (globs not resolved). Empty array if none. + * @throws Error if `pkgJson` is not an object or workspaces/packages contain non-strings. + * @public + */ +export function getWorkspacesPatterns(pkgJson: unknown): string[] { + if (typeof pkgJson !== 'object' || pkgJson === null) { + throw new Error('pkgJson must be an object'); + } + if (!('workspaces' in pkgJson)) { + return []; + } + + const workspaces = pkgJson.workspaces; + if (Array.isArray(workspaces)) { + assertStringArray(workspaces, 'pkgJson.workspaces'); + return workspaces; + } + if ( + typeof workspaces === 'object' && + workspaces !== null && + 'packages' in workspaces && + Array.isArray(workspaces.packages) + ) { + assertStringArray(workspaces.packages, 'pkgJson.workspaces.packages'); + return workspaces.packages; + } + return []; +} diff --git a/packages/cli-node/src/pacman/PackageManager.test.ts b/packages/cli-node/src/pacman/PackageManager.test.ts index fd7ace5bcb..e394d2ed60 100644 --- a/packages/cli-node/src/pacman/PackageManager.test.ts +++ b/packages/cli-node/src/pacman/PackageManager.test.ts @@ -48,7 +48,7 @@ describe('PackageManager', () => { 'package.json': JSON.stringify({ name: 'foo', workspaces: { - packages: [], + packages: ['packages/*'], }, }), }); @@ -59,7 +59,7 @@ describe('PackageManager', () => { mockDir.setContent({ 'package.json': JSON.stringify({ name: 'foo', - workspaces: [], + workspaces: ['packages/*'], }), }); await detectPackageManager();