Skip to content

Commit edde1be

Browse files
committed
refactor(@angular/cli): restrict MCP host process spawning to Angular CLI executable
Update the Host abstraction inside the Model Context Protocol (MCP) layer to tighten the system shell surface and improve semantics. The generic spawn and execute methods are replaced with specialized counterparts that default to the Angular CLI, enabling stronger path security containment for developers while also clarifying the distinct control flows needed for buffered discrete commands and long-running background services.
1 parent c9f4081 commit edde1be

11 files changed

Lines changed: 58 additions & 78 deletions

File tree

packages/angular/cli/src/commands/mcp/devserver.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export class LocalDevserver implements Devserver {
118118

119119
args.push(`--port=${this.port}`);
120120

121-
this.devserverProcess = this.host.spawn('ng', args, {
121+
this.devserverProcess = this.host.startNgProcess(args, {
122122
stdio: 'pipe',
123123
cwd: this.workspacePath,
124124
});

packages/angular/cli/src/commands/mcp/host.ts

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,11 @@ export interface Host {
7474
/**
7575
* Spawns a child process and returns a promise that resolves with the process's
7676
* output or rejects with a structured error.
77-
* @param command The command to run.
7877
* @param args The arguments to pass to the command.
7978
* @param options Options for the child process.
8079
* @returns A promise that resolves with the standard output and standard error of the command.
8180
*/
82-
runCommand(
83-
command: string,
81+
executeNgCommand(
8482
args: readonly string[],
8583
options?: {
8684
timeout?: number;
@@ -92,13 +90,11 @@ export interface Host {
9290

9391
/**
9492
* Spawns a long-running child process and returns the `ChildProcess` object.
95-
* @param command The command to run.
9693
* @param args The arguments to pass to the command.
9794
* @param options Options for the child process.
9895
* @returns The spawned `ChildProcess` instance.
9996
*/
100-
spawn(
101-
command: string,
97+
startNgProcess(
10298
args: readonly string[],
10399
options?: {
104100
stdio?: 'pipe' | 'ignore';
@@ -123,13 +119,13 @@ export interface Host {
123119
setRoots(roots: string[]): void;
124120
}
125121

126-
function resolveCommand(
127-
command: string,
122+
function resolveNgCommand(
128123
args: readonly string[],
129124
cwd?: string,
130125
): { command: string; args: readonly string[] } {
131-
if (command !== 'ng' || !cwd) {
132-
return { command, args };
126+
const defaultCommand = { command: 'ng', args };
127+
if (!cwd) {
128+
return defaultCommand;
133129
}
134130

135131
try {
@@ -150,7 +146,7 @@ function resolveCommand(
150146
// Failed to resolve the CLI binary, fall back to assuming `ng` is on PATH.
151147
}
152148

153-
return { command, args };
149+
return defaultCommand;
154150
}
155151

156152
/**
@@ -170,8 +166,7 @@ export const LocalWorkspaceHost: Host = {
170166
return nodeGlob(pattern, { ...options, withFileTypes: true });
171167
},
172168

173-
runCommand: async (
174-
command: string,
169+
executeNgCommand: async (
175170
args: readonly string[],
176171
options: {
177172
timeout?: number;
@@ -180,7 +175,7 @@ export const LocalWorkspaceHost: Host = {
180175
env?: Record<string, string>;
181176
} = {},
182177
): Promise<{ logs: string[] }> => {
183-
const resolved = resolveCommand(command, args, options.cwd);
178+
const resolved = resolveNgCommand(args, options.cwd);
184179
const signal = options.timeout ? AbortSignal.timeout(options.timeout) : undefined;
185180

186181
return new Promise((resolve, reject) => {
@@ -221,16 +216,15 @@ export const LocalWorkspaceHost: Host = {
221216
});
222217
},
223218

224-
spawn(
225-
command: string,
219+
startNgProcess(
226220
args: readonly string[],
227221
options: {
228222
stdio?: 'pipe' | 'ignore';
229223
cwd?: string;
230224
env?: Record<string, string>;
231225
} = {},
232226
): ChildProcess {
233-
const resolved = resolveCommand(command, args, options.cwd);
227+
const resolved = resolveNgCommand(args, options.cwd);
234228

235229
return spawn(resolved.command, resolved.args, {
236230
shell: false,
@@ -370,23 +364,17 @@ export function createRootRestrictedHost(
370364

371365
return baseHost.glob(pattern, options);
372366
},
373-
runCommand(command: string, args: readonly string[], options: { cwd?: string } = {}) {
374-
const effectiveCwd = options.cwd ?? process.cwd();
367+
executeNgCommand(args: readonly string[], options: Parameters<Host['executeNgCommand']>[1] = {}) {
368+
const effectiveCwd = options?.cwd ?? process.cwd();
375369
checkPath(effectiveCwd);
376-
if (command.includes('/') || command.includes('\\')) {
377-
checkPath(resolve(effectiveCwd, command));
378-
}
379370

380-
return baseHost.runCommand(command, args, options);
371+
return baseHost.executeNgCommand(args, options);
381372
},
382-
spawn(command: string, args: readonly string[], options: { cwd?: string } = {}) {
383-
const effectiveCwd = options.cwd ?? process.cwd();
373+
startNgProcess(args: readonly string[], options: Parameters<Host['startNgProcess']>[1] = {}) {
374+
const effectiveCwd = options?.cwd ?? process.cwd();
384375
checkPath(effectiveCwd);
385-
if (command.includes('/') || command.includes('\\')) {
386-
checkPath(resolve(effectiveCwd, command));
387-
}
388376

389-
return baseHost.spawn(command, args, options);
377+
return baseHost.startNgProcess(args, options);
390378
},
391379
};
392380
}

packages/angular/cli/src/commands/mcp/testing/mock-host.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ import type { Host } from '../host';
1313
* This class allows spying on host methods and controlling their return values.
1414
*/
1515
export class MockHost implements Host {
16-
runCommand = jasmine.createSpy('runCommand').and.resolveTo({ logs: [] });
16+
executeNgCommand = jasmine.createSpy('executeNgCommand').and.resolveTo({ logs: [] });
1717
stat = jasmine.createSpy('stat');
1818
existsSync = jasmine.createSpy('existsSync');
1919
readFile = jasmine.createSpy('readFile').and.resolveTo('');
2020
glob = jasmine.createSpy('glob').and.returnValue((async function* () {})());
21-
spawn = jasmine.createSpy('spawn');
21+
startNgProcess = jasmine.createSpy('startNgProcess');
2222
getAvailablePort = jasmine.createSpy('getAvailablePort');
2323
isPortAvailable = jasmine.createSpy('isPortAvailable').and.resolveTo(true);
2424
setRoots = jasmine.createSpy('setRoots');

packages/angular/cli/src/commands/mcp/testing/test-utils.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ import { MockHost } from './mock-host';
2020
*/
2121
export function createMockHost(): MockHost {
2222
return {
23-
runCommand: jasmine.createSpy<Host['runCommand']>('runCommand').and.resolveTo({ logs: [] }),
23+
executeNgCommand: jasmine
24+
.createSpy<Host['executeNgCommand']>('executeNgCommand')
25+
.and.resolveTo({ logs: [] }),
2426
stat: jasmine.createSpy<Host['stat']>('stat'),
2527
existsSync: jasmine.createSpy<Host['existsSync']>('existsSync'),
26-
spawn: jasmine.createSpy<Host['spawn']>('spawn'),
28+
startNgProcess: jasmine.createSpy<Host['startNgProcess']>('startNgProcess'),
2729
getAvailablePort: jasmine
2830
.createSpy<Host['getAvailablePort']>('getAvailablePort')
2931
.and.resolveTo(0),

packages/angular/cli/src/commands/mcp/tools/build.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export async function runBuild(input: BuildToolInput, context: McpToolContext) {
5252
let outputPath: string | undefined;
5353

5454
try {
55-
logs = (await context.host.runCommand('ng', args, { cwd: workspacePath })).logs;
55+
logs = (await context.host.executeNgCommand(args, { cwd: workspacePath })).logs;
5656
} catch (e) {
5757
status = 'failure';
5858
logs = getCommandErrorLogs(e);

packages/angular/cli/src/commands/mcp/tools/build_spec.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ describe('Build Tool', () => {
2929
it('should construct the command correctly with default configuration', async () => {
3030
mockContext.workspace.extensions['defaultProject'] = 'my-app';
3131
await runBuild({}, mockContext);
32-
expect(mockHost.runCommand).toHaveBeenCalledWith(
33-
'ng',
32+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(
3433
['build', 'my-app', '-c', 'development'],
3534
{ cwd: '/test' },
3635
);
@@ -39,8 +38,7 @@ describe('Build Tool', () => {
3938
it('should construct the command correctly with a specified project', async () => {
4039
addProjectToWorkspace(mockContext.workspace.projects, 'another-app');
4140
await runBuild({ project: 'another-app' }, mockContext);
42-
expect(mockHost.runCommand).toHaveBeenCalledWith(
43-
'ng',
41+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(
4442
['build', 'another-app', '-c', 'development'],
4543
{ cwd: '/test' },
4644
);
@@ -49,7 +47,7 @@ describe('Build Tool', () => {
4947
it('should construct the command correctly for a custom configuration', async () => {
5048
mockContext.workspace.extensions['defaultProject'] = 'my-app';
5149
await runBuild({ configuration: 'myconfig' }, mockContext);
52-
expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['build', 'my-app', '-c', 'myconfig'], {
50+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(['build', 'my-app', '-c', 'myconfig'], {
5351
cwd: '/test',
5452
});
5553
});
@@ -61,14 +59,13 @@ describe('Build Tool', () => {
6159
'some warning',
6260
'Output location: dist/my-app',
6361
];
64-
mockHost.runCommand.and.resolveTo({
62+
mockHost.executeNgCommand.and.resolveTo({
6563
logs: buildLogs,
6664
});
6765

6866
const { structuredContent } = await runBuild({ project: 'my-app' }, mockContext);
6967

70-
expect(mockHost.runCommand).toHaveBeenCalledWith(
71-
'ng',
68+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(
7269
['build', 'my-app', '-c', 'development'],
7370
{ cwd: '/test' },
7471
);
@@ -81,15 +78,14 @@ describe('Build Tool', () => {
8178
addProjectToWorkspace(mockContext.workspace.projects, 'my-failed-app');
8279
const buildLogs = ['Some output before the crash.', 'Error: Something went wrong!'];
8380
const error = new CommandError('Build failed', buildLogs, 1);
84-
mockHost.runCommand.and.rejectWith(error);
81+
mockHost.executeNgCommand.and.rejectWith(error);
8582

8683
const { structuredContent } = await runBuild(
8784
{ project: 'my-failed-app', configuration: 'production' },
8885
mockContext,
8986
);
9087

91-
expect(mockHost.runCommand).toHaveBeenCalledWith(
92-
'ng',
88+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(
9389
['build', 'my-failed-app', '-c', 'production'],
9490
{ cwd: '/test' },
9591
);
@@ -100,7 +96,7 @@ describe('Build Tool', () => {
10096

10197
it('should handle builds where the output path is not found in logs', async () => {
10298
const buildLogs = ["Some logs that don't match any output path."];
103-
mockHost.runCommand.and.resolveTo({ logs: buildLogs });
99+
mockHost.executeNgCommand.and.resolveTo({ logs: buildLogs });
104100

105101
mockContext.workspace.extensions['defaultProject'] = 'my-app';
106102
const { structuredContent } = await runBuild({}, mockContext);

packages/angular/cli/src/commands/mcp/tools/devserver/devserver_spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('Serve Tools', () => {
3939
mockContext = mock.context;
4040

4141
// Customize host spies
42-
mockHost.spawn.and.returnValue(mockProcess as unknown as ChildProcess);
42+
mockHost.startNgProcess.and.returnValue(mockProcess as unknown as ChildProcess);
4343
mockHost.getAvailablePort.and.callFake(() => Promise.resolve(portCounter++));
4444

4545
// Setup default project
@@ -52,7 +52,7 @@ describe('Serve Tools', () => {
5252
expect(startResult.structuredContent.message).toBe(
5353
`Development server for project 'my-app' started and watching for workspace changes.`,
5454
);
55-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'my-app', '--port=12345'], {
55+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'my-app', '--port=12345'], {
5656
stdio: 'pipe',
5757
cwd: '/test',
5858
});
@@ -69,7 +69,7 @@ describe('Serve Tools', () => {
6969
expect(startResult.structuredContent.message).toBe(
7070
`Development server for project 'my-app' started and watching for workspace changes.`,
7171
);
72-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'my-app', '--port=54321'], {
72+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'my-app', '--port=54321'], {
7373
stdio: 'pipe',
7474
cwd: '/test',
7575
});
@@ -125,17 +125,17 @@ describe('Serve Tools', () => {
125125

126126
// Start server for project 2, returning a new mock process.
127127
const process2 = new MockChildProcess();
128-
mockHost.spawn.and.returnValue(process2 as unknown as ChildProcess);
128+
mockHost.startNgProcess.and.returnValue(process2 as unknown as ChildProcess);
129129
const startResult2 = await startDevserver({ project: 'app-two' }, mockContext);
130130
expect(startResult2.structuredContent.message).toBe(
131131
`Development server for project 'app-two' started and watching for workspace changes.`,
132132
);
133133

134-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'app-one', '--port=12345'], {
134+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'app-one', '--port=12345'], {
135135
stdio: 'pipe',
136136
cwd: '/test',
137137
});
138-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'app-two', '--port=12346'], {
138+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'app-two', '--port=12346'], {
139139
stdio: 'pipe',
140140
cwd: '/test',
141141
});

packages/angular/cli/src/commands/mcp/tools/e2e.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export async function runE2e(input: E2eToolInput, host: Host, context: McpToolCo
6262
let logs: string[];
6363

6464
try {
65-
logs = (await host.runCommand('ng', args, { cwd: workspacePath })).logs;
65+
logs = (await host.executeNgCommand(args, { cwd: workspacePath })).logs;
6666
} catch (e) {
6767
status = 'failure';
6868
logs = getCommandErrorLogs(e);

packages/angular/cli/src/commands/mcp/tools/e2e_spec.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ describe('E2E Tool', () => {
3333
mockContext.workspace.extensions['defaultProject'] = 'my-app';
3434

3535
await runE2e({}, mockHost, mockContext);
36-
expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['e2e', 'my-app'], { cwd: '/test' });
36+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(['e2e', 'my-app'], { cwd: '/test' });
3737
});
3838

3939
it('should construct the command correctly with a specified project', async () => {
4040
addProjectToWorkspace(mockProjects, 'my-app', { e2e: { builder: 'mock-builder' } });
4141

4242
await runE2e({ project: 'my-app' }, mockHost, mockContext);
43-
expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['e2e', 'my-app'], { cwd: '/test' });
43+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(['e2e', 'my-app'], { cwd: '/test' });
4444
});
4545

4646
it('should error if project does not have e2e target', async () => {
@@ -50,7 +50,7 @@ describe('E2E Tool', () => {
5050

5151
expect(structuredContent.status).toBe('failure');
5252
expect(structuredContent.logs?.[0]).toContain("No e2e target is defined for project 'my-app'");
53-
expect(mockHost.runCommand).not.toHaveBeenCalled();
53+
expect(mockHost.executeNgCommand).not.toHaveBeenCalled();
5454
});
5555

5656
it('should error if no project was specified and the default project does not have e2e target', async () => {
@@ -61,40 +61,40 @@ describe('E2E Tool', () => {
6161

6262
expect(structuredContent.status).toBe('failure');
6363
expect(structuredContent.logs?.[0]).toContain("No e2e target is defined for project 'my-app'");
64-
expect(mockHost.runCommand).not.toHaveBeenCalled();
64+
expect(mockHost.executeNgCommand).not.toHaveBeenCalled();
6565
});
6666

6767
it('should handle a successful e2e run with a specified project', async () => {
6868
addProjectToWorkspace(mockProjects, 'my-app', { e2e: { builder: 'mock-builder' } });
6969
const e2eLogs = ['E2E passed for my-app'];
70-
mockHost.runCommand.and.resolveTo({ logs: e2eLogs });
70+
mockHost.executeNgCommand.and.resolveTo({ logs: e2eLogs });
7171

7272
const { structuredContent } = await runE2e({ project: 'my-app' }, mockHost, mockContext);
7373

7474
expect(structuredContent.status).toBe('success');
7575
expect(structuredContent.logs).toEqual(e2eLogs);
76-
expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['e2e', 'my-app'], { cwd: '/test' });
76+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(['e2e', 'my-app'], { cwd: '/test' });
7777
});
7878

7979
it('should handle a successful e2e run with the default project', async () => {
8080
mockContext.workspace.extensions['defaultProject'] = 'default-app';
8181
addProjectToWorkspace(mockProjects, 'default-app', { e2e: { builder: 'mock-builder' } });
8282
const e2eLogs = ['E2E passed for default-app'];
83-
mockHost.runCommand.and.resolveTo({ logs: e2eLogs });
83+
mockHost.executeNgCommand.and.resolveTo({ logs: e2eLogs });
8484

8585
const { structuredContent } = await runE2e({}, mockHost, mockContext);
8686

8787
expect(structuredContent.status).toBe('success');
8888
expect(structuredContent.logs).toEqual(e2eLogs);
89-
expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['e2e', 'default-app'], {
89+
expect(mockHost.executeNgCommand).toHaveBeenCalledWith(['e2e', 'default-app'], {
9090
cwd: '/test',
9191
});
9292
});
9393

9494
it('should handle a failed e2e run', async () => {
9595
addProjectToWorkspace(mockProjects, 'my-app', { e2e: { builder: 'mock-builder' } });
9696
const e2eLogs = ['E2E failed'];
97-
mockHost.runCommand.and.rejectWith(new CommandError('Failed', e2eLogs, 1));
97+
mockHost.executeNgCommand.and.rejectWith(new CommandError('Failed', e2eLogs, 1));
9898

9999
const { structuredContent } = await runE2e({ project: 'my-app' }, mockHost, mockContext);
100100

packages/angular/cli/src/commands/mcp/tools/test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export async function runTest(input: TestToolInput, context: McpToolContext) {
6565
let logs: string[];
6666

6767
try {
68-
logs = (await context.host.runCommand('ng', args, { cwd: workspacePath })).logs;
68+
logs = (await context.host.executeNgCommand(args, { cwd: workspacePath })).logs;
6969
} catch (e) {
7070
status = 'failure';
7171
logs = getCommandErrorLogs(e);

0 commit comments

Comments
 (0)