Skip to content

Commit 5ed1576

Browse files
committed
fix(@angular/cli): quote registry args on Windows and reject shell metacharacters
1 parent 9ac45f6 commit 5ed1576

3 files changed

Lines changed: 107 additions & 2 deletions

File tree

packages/angular/cli/src/commands/add/cli.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,18 @@ import { VERSION } from '../../utilities/version';
3232

3333
class CommandError extends Error {}
3434

35+
export const SHELL_METACHARACTERS = /[&|;$`()]/;
36+
37+
export function validateRegistry(registry: string): void {
38+
if (!URL.canParse(registry)) {
39+
throw new CommandModuleError('Option --registry must be a valid URL.');
40+
}
41+
42+
if (SHELL_METACHARACTERS.test(registry)) {
43+
throw new CommandModuleError('Option --registry contains invalid characters.');
44+
}
45+
}
46+
3547
interface AddCommandArgs extends SchematicsCommandArgs {
3648
collection: string;
3749
verbose?: boolean;
@@ -132,7 +144,9 @@ export default class AddCommandModule
132144
return true;
133145
}
134146

135-
if (typeof registry === 'string' && URL.canParse(registry)) {
147+
if (typeof registry === 'string') {
148+
validateRegistry(registry);
149+
136150
return true;
137151
}
138152

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import { SHELL_METACHARACTERS, validateRegistry } from './cli';
10+
11+
describe('registry validation', () => {
12+
describe('SHELL_METACHARACTERS', () => {
13+
it('should match shell metacharacters', () => {
14+
expect(SHELL_METACHARACTERS.test('&')).toBe(true);
15+
expect(SHELL_METACHARACTERS.test('|')).toBe(true);
16+
expect(SHELL_METACHARACTERS.test(';')).toBe(true);
17+
expect(SHELL_METACHARACTERS.test('$')).toBe(true);
18+
expect(SHELL_METACHARACTERS.test('`')).toBe(true);
19+
expect(SHELL_METACHARACTERS.test('(')).toBe(true);
20+
expect(SHELL_METACHARACTERS.test(')')).toBe(true);
21+
});
22+
23+
it('should not match safe URL characters', () => {
24+
expect(SHELL_METACHARACTERS.test('https://registry.example.com')).toBe(false);
25+
expect(SHELL_METACHARACTERS.test('http://registry.example.com/path')).toBe(false);
26+
expect(SHELL_METACHARACTERS.test('https://registry.example.com:8080')).toBe(false);
27+
});
28+
});
29+
30+
describe('validateRegistry', () => {
31+
it('should reject URLs with shell metacharacters', () => {
32+
expect(() => validateRegistry('https://example.com&cmd')).toThrow(
33+
'Option --registry contains invalid characters.',
34+
);
35+
expect(() => validateRegistry('https://example.com|cmd')).toThrow(
36+
'Option --registry contains invalid characters.',
37+
);
38+
expect(() => validateRegistry('https://example.com;cmd')).toThrow(
39+
'Option --registry contains invalid characters.',
40+
);
41+
expect(() => validateRegistry('https://example.com$cmd')).toThrow(
42+
'Option --registry contains invalid characters.',
43+
);
44+
expect(() => validateRegistry('https://example.com`cmd`')).toThrow(
45+
'Option --registry contains invalid characters.',
46+
);
47+
expect(() => validateRegistry('https://example.com(cmd)')).toThrow(
48+
'Option --registry contains invalid characters.',
49+
);
50+
});
51+
52+
it('should accept valid URLs', () => {
53+
expect(() => validateRegistry('https://registry.example.com')).not.toThrow();
54+
expect(() => validateRegistry('http://registry.example.com:8080')).not.toThrow();
55+
expect(() => validateRegistry('https://registry.example.com/path')).not.toThrow();
56+
});
57+
58+
it('should reject invalid URLs', () => {
59+
expect(() => validateRegistry('not-a-url')).toThrow(
60+
'Option --registry must be a valid URL.',
61+
);
62+
});
63+
});
64+
65+
describe('Windows shell quoting', () => {
66+
it('should wrap args in double quotes', () => {
67+
const command = 'npm';
68+
const args = ['--registry', 'https://registry.example.com'];
69+
const result = `${command} ${args
70+
.map((a) => `"${String(a).replace(/"/g, '\\"')}"`)
71+
.join(' ')}`;
72+
expect(result).toBe(
73+
'npm "--registry" "https://registry.example.com"',
74+
);
75+
});
76+
77+
it('should escape inner double quotes', () => {
78+
const command = 'npm';
79+
const args = ['--registry', 'https://example.com?key="value"'];
80+
const result = `${command} ${args
81+
.map((a) => `"${String(a).replace(/"/g, '\\"')}"`)
82+
.join(' ')}`;
83+
expect(result).toBe(
84+
'npm "--registry" "https://example.com?key=\\"value\\""',
85+
);
86+
});
87+
});
88+
});

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,10 @@ export const NodeJS_HOST: Host = {
158158
env,
159159
} satisfies SpawnOptions;
160160
const childProcess = isWin32
161-
? spawn(`${command} ${args.join(' ')}`, spawnOptions)
161+
? spawn(
162+
`${command} ${args.map((a) => `"${String(a).replace(/"/g, '\\"')}"`).join(' ')}`,
163+
spawnOptions,
164+
)
162165
: spawn(command, args, spawnOptions);
163166

164167
let stdout = '';

0 commit comments

Comments
 (0)