From d15b67630f65704874ae76c769192fbc791d8238 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Wed, 29 Apr 2026 06:30:15 +0000 Subject: [PATCH] fix(@angular/cli): restrict MCP workspace access to allowed client roots during resolution Introduces validation during workspace and project resolution to ensure the resolved workspace path falls within the allowed MCP roots defined by client capabilities. Throws an error if the workspace resolves outside the allowed roots. Closes #33077 --- .../cli/src/commands/mcp/tools/build.ts | 1 + .../mcp/tools/devserver/devserver-start.ts | 1 + .../mcp/tools/devserver/devserver-stop.ts | 1 + .../devserver/devserver-wait-for-build.ts | 1 + .../angular/cli/src/commands/mcp/tools/e2e.ts | 1 + .../cli/src/commands/mcp/tools/modernize.ts | 1 + .../cli/src/commands/mcp/workspace-utils.ts | 61 +++++++++++++++++- .../src/commands/mcp/workspace-utils_spec.ts | 64 +++++++++++++++++++ 8 files changed, 130 insertions(+), 1 deletion(-) diff --git a/packages/angular/cli/src/commands/mcp/tools/build.ts b/packages/angular/cli/src/commands/mcp/tools/build.ts index 45d3765b3c86..a04812f8544b 100644 --- a/packages/angular/cli/src/commands/mcp/tools/build.ts +++ b/packages/angular/cli/src/commands/mcp/tools/build.ts @@ -38,6 +38,7 @@ export type BuildToolOutput = z.infer; export async function runBuild(input: BuildToolInput, context: McpToolContext) { const { workspacePath, projectName } = await resolveWorkspaceAndProject({ host: context.host, + server: context.server, workspacePathInput: input.workspace, projectNameInput: input.project, mcpWorkspace: context.workspace, diff --git a/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-start.ts b/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-start.ts index f5f413cfad30..8f5548f14019 100644 --- a/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-start.ts +++ b/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-start.ts @@ -45,6 +45,7 @@ function localhostAddress(port: number) { export async function startDevserver(input: DevserverStartToolInput, context: McpToolContext) { const { workspacePath, projectName } = await resolveWorkspaceAndProject({ host: context.host, + server: context.server, workspacePathInput: input.workspace, projectNameInput: input.project, mcpWorkspace: context.workspace, diff --git a/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-stop.ts b/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-stop.ts index 64991bc5adb3..1c90bd9ecd98 100644 --- a/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-stop.ts +++ b/packages/angular/cli/src/commands/mcp/tools/devserver/devserver-stop.ts @@ -29,6 +29,7 @@ export type DevserverStopToolOutput = z.infer; export async function runE2e(input: E2eToolInput, host: Host, context: McpToolContext) { const { workspacePath, workspace, projectName } = await resolveWorkspaceAndProject({ host, + server: context.server, workspacePathInput: input.workspace, projectNameInput: input.project, mcpWorkspace: context.workspace, diff --git a/packages/angular/cli/src/commands/mcp/tools/modernize.ts b/packages/angular/cli/src/commands/mcp/tools/modernize.ts index 871622d76390..d56d2c30edfd 100644 --- a/packages/angular/cli/src/commands/mcp/tools/modernize.ts +++ b/packages/angular/cli/src/commands/mcp/tools/modernize.ts @@ -108,6 +108,7 @@ export async function runModernization(input: ModernizeInput, context: McpToolCo const { workspacePath, projectName } = await resolveWorkspaceAndProject({ host: context.host, + server: context.server, workspacePathInput: input.workspace, projectNameInput: input.project, mcpWorkspace: context.workspace, diff --git a/packages/angular/cli/src/commands/mcp/workspace-utils.ts b/packages/angular/cli/src/commands/mcp/workspace-utils.ts index d1edf55fa56b..6cc245ff1dbc 100644 --- a/packages/angular/cli/src/commands/mcp/workspace-utils.ts +++ b/packages/angular/cli/src/commands/mcp/workspace-utils.ts @@ -7,7 +7,9 @@ */ import { workspaces } from '@angular-devkit/core'; -import { dirname, join } from 'node:path'; +import { realpathSync } from 'node:fs'; +import { dirname, isAbsolute, join, normalize, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { AngularWorkspace } from '../../utilities/config'; import { type Host, LocalWorkspaceHost } from './host'; import { McpToolContext } from './tools/tool-registry'; @@ -80,6 +82,44 @@ export function getDefaultProjectName(workspace: AngularWorkspace | undefined): return undefined; } +function isWithinAllowedRoot(root: string, targetPath: string): boolean { + const rel = relative(root, targetPath); + + return !rel.startsWith('..') && !isAbsolute(rel); +} + +async function getAllowedWorkspaceRoots(server: McpToolContext['server']): Promise { + let roots: string[]; + const clientCapabilities = server.server.getClientCapabilities(); + + if (clientCapabilities?.roots) { + const { roots: clientRoots } = await server.server.listRoots(); + roots = clientRoots?.map((root) => fileURLToPath(root.uri)) ?? []; + } else { + roots = [process.cwd()]; + } + + return roots + .map((root) => { + try { + return realpathSync(root); + } catch { + return null; + } + }) + .filter((root): root is string => root !== null); +} + +async function isAllowedWorkspacePath( + server: McpToolContext['server'], + workspacePath: string, +): Promise { + const allowedRoots = await getAllowedWorkspaceRoots(server); + const resolvedWorkspacePath = realpathSync(workspacePath); + + return allowedRoots.some((root) => isWithinAllowedRoot(root, resolvedWorkspacePath)); +} + /** * Resolves workspace and project for tools to operate on. * @@ -89,11 +129,13 @@ export function getDefaultProjectName(workspace: AngularWorkspace | undefined): */ export async function resolveWorkspaceAndProject({ host, + server, workspacePathInput, projectNameInput, mcpWorkspace, }: { host: Host; + server?: McpToolContext['server']; workspacePathInput?: string; projectNameInput?: string; mcpWorkspace?: AngularWorkspace; @@ -118,6 +160,15 @@ export async function resolveWorkspaceAndProject({ "You can use 'list_projects' to find available workspaces.", ); } + if (server) { + if (!(await isAllowedWorkspacePath(server, workspacePathInput))) { + throw new Error( + `Workspace path is outside the allowed MCP roots: ${workspacePathInput}. ` + + "You can use 'list_projects' to find available workspaces.", + ); + } + } + workspacePath = workspacePathInput; const configPath = join(workspacePath, 'angular.json'); try { @@ -137,6 +188,14 @@ export async function resolveWorkspaceAndProject({ "You can use 'list_projects' to find available workspaces.", ); } + + if (server && !(await isAllowedWorkspacePath(server, found))) { + throw new Error( + `The current directory resolves to a workspace outside the allowed MCP roots: ${found}. ` + + "You can use 'list_projects' to find available workspaces.", + ); + } + workspacePath = found; const configPath = join(workspacePath, 'angular.json'); try { diff --git a/packages/angular/cli/src/commands/mcp/workspace-utils_spec.ts b/packages/angular/cli/src/commands/mcp/workspace-utils_spec.ts index 62e8df3100e8..a000dd01da34 100644 --- a/packages/angular/cli/src/commands/mcp/workspace-utils_spec.ts +++ b/packages/angular/cli/src/commands/mcp/workspace-utils_spec.ts @@ -7,7 +7,10 @@ */ import { workspaces } from '@angular-devkit/core'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; import { AngularWorkspace } from '../../utilities/config'; import { LocalWorkspaceHost } from './host'; import { addProjectToWorkspace, createMockContext, createMockHost } from './testing/test-utils'; @@ -101,11 +104,24 @@ describe('MCP Workspace Utils', () => { describe('resolveWorkspaceAndProject', () => { let mockHost: ReturnType; let mockWorkspace: AngularWorkspace; + let mockServer: NonNullable[0]['server']>; + let tempDir: string; + let allowedRoot: string; + let allowedWorkspace: string; + let outsideWorkspace: string; const cwd = './'; beforeEach(() => { mockHost = createMockHost(); spyOn(process, 'cwd').and.returnValue(cwd); + tempDir = mkdtempSync(join(tmpdir(), 'mcp-workspace-utils-')); + allowedRoot = join(tempDir, 'allowed-root'); + allowedWorkspace = join(allowedRoot, 'workspace'); + outsideWorkspace = join(tempDir, 'outside-workspace'); + mkdirSync(allowedWorkspace, { recursive: true }); + mkdirSync(outsideWorkspace, { recursive: true }); + writeFileSync(join(allowedWorkspace, 'angular.json'), '{}'); + writeFileSync(join(outsideWorkspace, 'angular.json'), '{}'); // Setup default mocks mockHost.existsSync.and.callFake((p) => { @@ -120,6 +136,18 @@ describe('MCP Workspace Utils', () => { if (p === '/my/workspace/angular.json') { return true; } + if (p === allowedWorkspace) { + return true; + } + if (p === join(allowedWorkspace, 'angular.json')) { + return true; + } + if (p === outsideWorkspace) { + return true; + } + if (p === join(outsideWorkspace, 'angular.json')) { + return true; + } return false; }); @@ -139,6 +167,21 @@ describe('MCP Workspace Utils', () => { } as unknown as AngularWorkspace; spyOn(AngularWorkspace, 'load').and.resolveTo(mockWorkspace); + + mockServer = { + server: { + getClientCapabilities: jasmine.createSpy('getClientCapabilities').and.returnValue({ + roots: { listChanged: false }, + }), + listRoots: jasmine.createSpy('listRoots').and.resolveTo({ + roots: [{ uri: pathToFileURL(allowedRoot).href, name: 'allowed-root' }], + }), + }, + } as unknown as NonNullable[0]['server']>; + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); }); it('should resolve workspace from CWD if not provided and mcpWorkspace is absent', async () => { @@ -179,6 +222,27 @@ describe('MCP Workspace Utils', () => { expect(AngularWorkspace.load).toHaveBeenCalledWith('/my/workspace/angular.json'); }); + it('should allow provided workspace within allowed MCP roots', async () => { + const result = await resolveWorkspaceAndProject({ + host: mockHost, + server: mockServer, + workspacePathInput: allowedWorkspace, + }); + expect(result.workspacePath).toBe(allowedWorkspace); + expect(AngularWorkspace.load).toHaveBeenCalledWith(join(allowedWorkspace, 'angular.json')); + expect(mockServer.server.listRoots).toHaveBeenCalled(); + }); + + it('should reject provided workspace outside allowed MCP roots', async () => { + await expectAsync( + resolveWorkspaceAndProject({ + host: mockHost, + server: mockServer, + workspacePathInput: outsideWorkspace, + }), + ).toBeRejectedWithError(/Workspace path is outside the allowed MCP roots/); + }); + it('should throw if provided workspace does not exist', async () => { mockHost.existsSync.and.returnValue(false); await expectAsync(