Skip to content

Commit 4a78ddc

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 4a78ddc

2 files changed

Lines changed: 99 additions & 2 deletions

File tree

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

Lines changed: 37 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,18 @@ 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 typeof name === 'string' && typeof version === 'string' && valid(version) !== null;
230+
}
231+
220232
/**
221233
* Parses the output of `npm view` or a compatible command to get a package manifest.
222234
* @param stdout The standard output of the command.
@@ -242,7 +254,10 @@ export function parseNpmLikeManifest(stdout: string, logger?: Logger): PackageMa
242254
let maxManifest: PackageManifest | null = null;
243255

244256
for (const manifest of result) {
245-
if (!manifest || typeof manifest.version !== 'string') {
257+
if (!isValidManifest(manifest)) {
258+
logger?.debug(
259+
' Skipping invalid manifest in array (missing name, version, or invalid SemVer).',
260+
);
246261
continue;
247262
}
248263

@@ -251,9 +266,21 @@ export function parseNpmLikeManifest(stdout: string, logger?: Logger): PackageMa
251266
}
252267
}
253268

269+
if (!maxManifest) {
270+
logger?.debug(' No valid manifests found in the array.');
271+
}
272+
254273
return maxManifest;
255274
}
256275

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

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

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

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

Lines changed: 62 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,72 @@ 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([{ name: 'foo' }, { version: '1.2.0' }]);
161+
expect(parseNpmLikeManifest(stdout)).toBeNull();
162+
});
163+
164+
it('should return null for invalid single object', () => {
165+
const stdout = JSON.stringify({ name: 'foo' }); // Missing version
166+
expect(parseNpmLikeManifest(stdout)).toBeNull();
167+
});
168+
169+
it('should skip manifests with invalid semver versions in an array', () => {
170+
const stdout = JSON.stringify([
171+
{ name: 'foo', version: '1.0.0' },
172+
{ name: 'foo', version: 'invalid-version' },
173+
{ name: 'foo', version: '1.0' },
174+
]);
175+
expect(parseNpmLikeManifest(stdout)).toEqual({ name: 'foo', version: '1.0.0' });
176+
});
177+
178+
it('should return null for single object with invalid semver version', () => {
179+
const stdout = JSON.stringify({ name: 'foo', version: 'invalid-version' });
180+
expect(parseNpmLikeManifest(stdout)).toBeNull();
181+
});
182+
147183
it('should return null for empty stdout', () => {
148184
expect(parseNpmLikeManifest('')).toBeNull();
149185
});
150186
});
151187

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

0 commit comments

Comments
 (0)