Skip to content

Commit 0ae7837

Browse files
clydinalan-agius4
authored andcommitted
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 373c6df commit 0ae7837

11 files changed

Lines changed: 61 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: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,11 @@ export interface Host {
7676
/**
7777
* Spawns a child process and returns a promise that resolves with the process's
7878
* output or rejects with a structured error.
79-
* @param command The command to run.
8079
* @param args The arguments to pass to the command.
8180
* @param options Options for the child process.
8281
* @returns A promise that resolves with the standard output and standard error of the command.
8382
*/
84-
runCommand(
85-
command: string,
83+
executeNgCommand(
8684
args: readonly string[],
8785
options?: {
8886
timeout?: number;
@@ -94,13 +92,11 @@ export interface Host {
9492

9593
/**
9694
* Spawns a long-running child process and returns the `ChildProcess` object.
97-
* @param command The command to run.
9895
* @param args The arguments to pass to the command.
9996
* @param options Options for the child process.
10097
* @returns The spawned `ChildProcess` instance.
10198
*/
102-
spawn(
103-
command: string,
99+
startNgProcess(
104100
args: readonly string[],
105101
options?: {
106102
stdio?: 'pipe' | 'ignore';
@@ -125,13 +121,13 @@ export interface Host {
125121
setRoots(roots: string[]): void;
126122
}
127123

128-
function resolveCommand(
129-
command: string,
124+
function resolveNgCommand(
130125
args: readonly string[],
131126
cwd?: string,
132127
): { command: string; args: readonly string[] } {
133-
if (command !== 'ng' || !cwd) {
134-
return { command, args };
128+
const defaultCommand = { command: 'ng', args };
129+
if (!cwd) {
130+
return defaultCommand;
135131
}
136132

137133
try {
@@ -152,7 +148,7 @@ function resolveCommand(
152148
// Failed to resolve the CLI binary, fall back to assuming `ng` is on PATH.
153149
}
154150

155-
return { command, args };
151+
return defaultCommand;
156152
}
157153

158154
/**
@@ -172,8 +168,7 @@ export const LocalWorkspaceHost: Host = {
172168
return nodeGlob(pattern, { ...options, withFileTypes: true });
173169
},
174170

175-
runCommand: async (
176-
command: string,
171+
executeNgCommand: async (
177172
args: readonly string[],
178173
options: {
179174
timeout?: number;
@@ -182,7 +177,7 @@ export const LocalWorkspaceHost: Host = {
182177
env?: Record<string, string>;
183178
} = {},
184179
): Promise<{ logs: string[] }> => {
185-
const resolved = resolveCommand(command, args, options.cwd);
180+
const resolved = resolveNgCommand(args, options.cwd);
186181
const signal = options.timeout ? AbortSignal.timeout(options.timeout) : undefined;
187182

188183
return new Promise((resolve, reject) => {
@@ -223,16 +218,15 @@ export const LocalWorkspaceHost: Host = {
223218
});
224219
},
225220

226-
spawn(
227-
command: string,
221+
startNgProcess(
228222
args: readonly string[],
229223
options: {
230224
stdio?: 'pipe' | 'ignore';
231225
cwd?: string;
232226
env?: Record<string, string>;
233227
} = {},
234228
): ChildProcess {
235-
const resolved = resolveCommand(command, args, options.cwd);
229+
const resolved = resolveNgCommand(args, options.cwd);
236230

237231
return spawn(resolved.command, resolved.args, {
238232
shell: false,
@@ -372,23 +366,20 @@ export function createRootRestrictedHost(
372366

373367
return baseHost.glob(pattern, options);
374368
},
375-
runCommand(command: string, args: readonly string[], options: { cwd?: string } = {}) {
376-
const effectiveCwd = options.cwd ?? process.cwd();
369+
executeNgCommand(
370+
args: readonly string[],
371+
options: Parameters<Host['executeNgCommand']>[1] = {},
372+
) {
373+
const effectiveCwd = options?.cwd ?? process.cwd();
377374
checkPath(effectiveCwd);
378-
if (command.includes('/') || command.includes('\\')) {
379-
checkPath(resolve(effectiveCwd, command));
380-
}
381375

382-
return baseHost.runCommand(command, args, options);
376+
return baseHost.executeNgCommand(args, options);
383377
},
384-
spawn(command: string, args: readonly string[], options: { cwd?: string } = {}) {
385-
const effectiveCwd = options.cwd ?? process.cwd();
378+
startNgProcess(args: readonly string[], options: Parameters<Host['startNgProcess']>[1] = {}) {
379+
const effectiveCwd = options?.cwd ?? process.cwd();
386380
checkPath(effectiveCwd);
387-
if (command.includes('/') || command.includes('\\')) {
388-
checkPath(resolve(effectiveCwd, command));
389-
}
390381

391-
return baseHost.spawn(command, args, options);
382+
return baseHost.startNgProcess(args, options);
392383
},
393384
};
394385
}

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
@@ -44,7 +44,7 @@ describe('Serve Tools', () => {
4444
mockContext = mock.context;
4545

4646
// Customize host spies
47-
mockHost.spawn.and.returnValue(mockProcess as unknown as ChildProcess);
47+
mockHost.startNgProcess.and.returnValue(mockProcess as unknown as ChildProcess);
4848
mockHost.getAvailablePort.and.callFake(() => Promise.resolve(portCounter++));
4949

5050
// Setup default project
@@ -57,7 +57,7 @@ describe('Serve Tools', () => {
5757
expect(startResult.structuredContent.message).toBe(
5858
`Development server for project 'my-app' started and watching for workspace changes.`,
5959
);
60-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'my-app', '--port=12345'], {
60+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'my-app', '--port=12345'], {
6161
stdio: 'pipe',
6262
cwd: '/test',
6363
});
@@ -74,7 +74,7 @@ describe('Serve Tools', () => {
7474
expect(startResult.structuredContent.message).toBe(
7575
`Development server for project 'my-app' started and watching for workspace changes.`,
7676
);
77-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'my-app', '--port=54321'], {
77+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'my-app', '--port=54321'], {
7878
stdio: 'pipe',
7979
cwd: '/test',
8080
});
@@ -130,17 +130,17 @@ describe('Serve Tools', () => {
130130

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

139-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'app-one', '--port=12345'], {
139+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'app-one', '--port=12345'], {
140140
stdio: 'pipe',
141141
cwd: '/test',
142142
});
143-
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'app-two', '--port=12346'], {
143+
expect(mockHost.startNgProcess).toHaveBeenCalledWith(['serve', 'app-two', '--port=12346'], {
144144
stdio: 'pipe',
145145
cwd: '/test',
146146
});

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)