Skip to content

Commit 4f0385f

Browse files
committed
refactor(@angular/cli): add validation and logging to npm manifest parsing
Introduce a reusable `isValidManifest` type guard to ensure that parsed manifests contain both `name` and `version` strings. This applies to both single object responses and elements within an array response from the package manager.
1 parent 5c1db4e commit 4f0385f

2 files changed

Lines changed: 100 additions & 2 deletions

File tree

packages/angular/cli/src/package-managers/parsers.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@
1212
* into their own file improves modularity and allows for focused testing.
1313
*/
1414

15+
import { compare, valid } from 'semver';
1516
import { ErrorInfo } from './error';
1617
import { Logger } from './logger';
1718
import { PackageManifest, PackageMetadata } from './package-metadata';
1819
import { InstalledPackage } from './package-tree';
19-
import { compare } from 'semver';
2020

2121
const MAX_LOG_LENGTH = 1024;
2222

@@ -217,6 +217,22 @@ export function parseYarnClassicDependencies(
217217
return dependencies;
218218
}
219219

220+
function isValidManifest(obj: unknown): obj is PackageManifest {
221+
if (typeof obj !== 'object' || obj === null) {
222+
return false;
223+
}
224+
225+
const record = obj as Record<string, unknown>;
226+
const name = record.name;
227+
const version = record.version;
228+
229+
return (
230+
typeof name === 'string' &&
231+
typeof version === 'string' &&
232+
valid(version) !== null
233+
);
234+
}
235+
220236
/**
221237
* Parses the output of `npm view` or a compatible command to get a package manifest.
222238
* @param stdout The standard output of the command.
@@ -242,7 +258,8 @@ export function parseNpmLikeManifest(stdout: string, logger?: Logger): PackageMa
242258
let maxManifest: PackageManifest | null = null;
243259

244260
for (const manifest of result) {
245-
if (!manifest || typeof manifest.version !== 'string') {
261+
if (!isValidManifest(manifest)) {
262+
logger?.debug(' Skipping invalid manifest in array (missing name, version, or invalid SemVer).');
246263
continue;
247264
}
248265

@@ -251,9 +268,19 @@ export function parseNpmLikeManifest(stdout: string, logger?: Logger): PackageMa
251268
}
252269
}
253270

271+
if (!maxManifest) {
272+
logger?.debug(' No valid manifests found in the array.');
273+
}
274+
254275
return maxManifest;
255276
}
256277

278+
if (!isValidManifest(result)) {
279+
logger?.debug(' Parsed JSON is not a valid manifest (missing name, version, or invalid SemVer).');
280+
281+
return null;
282+
}
283+
257284
return result;
258285
}
259286

@@ -329,6 +356,12 @@ export function parseYarnClassicManifest(stdout: string, logger?: Logger): Packa
329356
manifest['ng-add'].save ??= false;
330357
}
331358

359+
if (!isValidManifest(manifest)) {
360+
logger?.debug(' Parsed JSON is not a valid manifest (missing name, version, or invalid SemVer).');
361+
362+
return null;
363+
}
364+
332365
return manifest;
333366
}
334367

packages/angular/cli/src/package-managers/parsers_spec.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
parseNpmLikeManifest,
1414
parseYarnClassicDependencies,
1515
parseYarnClassicError,
16+
parseYarnClassicManifest,
1617
parseYarnModernDependencies,
1718
} from './parsers';
1819

@@ -144,11 +145,75 @@ describe('parsers', () => {
144145
expect(parseNpmLikeManifest(stdout)).toEqual({ name: 'foo', version: '1.1.0' });
145146
});
146147

148+
it('should skip invalid manifests in an array', () => {
149+
const stdout = JSON.stringify([
150+
{ name: 'foo', version: '1.0.0' },
151+
{ name: 'foo' }, // Missing version
152+
{ version: '1.2.0' }, // Missing name
153+
null,
154+
"invalid",
155+
]);
156+
expect(parseNpmLikeManifest(stdout)).toEqual({ name: 'foo', version: '1.0.0' });
157+
});
158+
159+
it('should return null if no valid manifests found in the array', () => {
160+
const stdout = JSON.stringify([
161+
{ name: 'foo' },
162+
{ version: '1.2.0' },
163+
]);
164+
expect(parseNpmLikeManifest(stdout)).toBeNull();
165+
});
166+
167+
it('should return null for invalid single object', () => {
168+
const stdout = JSON.stringify({ name: 'foo' }); // Missing version
169+
expect(parseNpmLikeManifest(stdout)).toBeNull();
170+
});
171+
172+
it('should skip manifests with invalid semver versions in an array', () => {
173+
const stdout = JSON.stringify([
174+
{ name: 'foo', version: '1.0.0' },
175+
{ name: 'foo', version: 'invalid-version' },
176+
{ name: 'foo', version: '1.0' },
177+
]);
178+
expect(parseNpmLikeManifest(stdout)).toEqual({ name: 'foo', version: '1.0.0' });
179+
});
180+
181+
it('should return null for single object with invalid semver version', () => {
182+
const stdout = JSON.stringify({ name: 'foo', version: 'invalid-version' });
183+
expect(parseNpmLikeManifest(stdout)).toBeNull();
184+
});
185+
147186
it('should return null for empty stdout', () => {
148187
expect(parseNpmLikeManifest('')).toBeNull();
149188
});
150189
});
151190

191+
describe('parseYarnClassicManifest', () => {
192+
it('should parse a valid manifest', () => {
193+
const stdout = JSON.stringify({
194+
type: 'inspect',
195+
data: { name: 'foo', version: '1.0.0' },
196+
});
197+
expect(parseYarnClassicManifest(stdout)).toEqual({ name: 'foo', version: '1.0.0' });
198+
});
199+
200+
it('should return null for invalid manifest', () => {
201+
const stdout = JSON.stringify({
202+
type: 'inspect',
203+
data: { name: 'foo' },
204+
});
205+
expect(parseYarnClassicManifest(stdout)).toBeNull();
206+
});
207+
208+
it('should return null if no inspect type found', () => {
209+
const stdout = JSON.stringify({
210+
type: 'other',
211+
data: { name: 'foo', version: '1.0.0' },
212+
});
213+
expect(parseYarnClassicManifest(stdout)).toBeNull();
214+
});
215+
});
216+
152217
describe('parseYarnClassicError', () => {
153218
it('should parse a 404 from verbose logs', () => {
154219
const stdout =

0 commit comments

Comments
 (0)