From a71bb318524eb33d2029671270f478412e902fdb Mon Sep 17 00:00:00 2001 From: Herdiyan Adam Putra Date: Sat, 25 Jul 2026 00:00:22 +0700 Subject: [PATCH 1/2] fix(@angular/cli): enforce MCP roots in get_best_practices tool getVersionSpecificBestPractices() resolves an npm package (and reads a file path declared in that package's package.json) starting from a caller-supplied workspacePath, without checking it against the client's declared MCP roots. Every other workspace-path-consuming MCP tool (run_target, devserver_start/stop/wait_for_build) validates this through resolveWorkspaceAndProject()'s isAllowedWorkspacePath() check; this tool never did. Export isAllowedWorkspacePath() from workspace-utils.ts and call it in getVersionSpecificBestPractices() before resolving anything, falling back to the bundled guide when the path is outside the allowed roots, matching this file's existing fallback behavior for every other failure case. --- .../src/commands/mcp/tools/best-practices.ts | 20 +++++++++++++++++-- .../cli/src/commands/mcp/workspace-utils.ts | 2 +- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/angular/cli/src/commands/mcp/tools/best-practices.ts b/packages/angular/cli/src/commands/mcp/tools/best-practices.ts index dca61eb700b3..3733a9c61641 100644 --- a/packages/angular/cli/src/commands/mcp/tools/best-practices.ts +++ b/packages/angular/cli/src/commands/mcp/tools/best-practices.ts @@ -20,6 +20,7 @@ import { createRequire } from 'node:module'; import { dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { z } from 'zod'; import { VERSION } from '../../../utilities/version'; +import { isAllowedWorkspacePath } from '../workspace-utils'; import { type McpToolContext, declareTool } from './tool-registry'; const bestPracticesInputSchema = z.object({ @@ -86,13 +87,24 @@ async function getBundledBestPractices(): Promise { * * @param workspacePath The absolute path to the user's `angular.json` file. * @param logger The MCP tool context logger for reporting warnings. + * @param server The MCP server context, used to enforce the client's declared roots. * @returns A promise that resolves to an object containing the guide's content and source, * or `undefined` if the guide could not be resolved. */ async function getVersionSpecificBestPractices( workspacePath: string, logger: McpToolContext['logger'], + server: McpToolContext['server'], ): Promise<{ content: string; source: string } | undefined> { + if (server && !(await isAllowedWorkspacePath(server, workspacePath))) { + logger.warn( + `Workspace path is outside the allowed MCP roots: ${workspacePath}. ` + + 'Falling back to the bundled guide.', + ); + + return undefined; + } + // 1. Resolve the path to package.json let pkgJsonPath: string; try { @@ -175,7 +187,7 @@ async function getVersionSpecificBestPractices( * @param context The MCP tool context, containing the logger. * @returns An async function that serves as the tool's executor. */ -function createBestPracticesHandler({ logger }: McpToolContext) { +function createBestPracticesHandler({ logger, server }: McpToolContext) { let bundledBestPractices: Promise; return async (input: BestPracticesInput) => { @@ -184,7 +196,11 @@ function createBestPracticesHandler({ logger }: McpToolContext) { // First, try to get the version-specific guide. if (input.workspacePath) { - const versionSpecific = await getVersionSpecificBestPractices(input.workspacePath, logger); + const versionSpecific = await getVersionSpecificBestPractices( + input.workspacePath, + logger, + server, + ); if (versionSpecific) { content = versionSpecific.content; source = versionSpecific.source; diff --git a/packages/angular/cli/src/commands/mcp/workspace-utils.ts b/packages/angular/cli/src/commands/mcp/workspace-utils.ts index 6cc245ff1dbc..94c4c55655c0 100644 --- a/packages/angular/cli/src/commands/mcp/workspace-utils.ts +++ b/packages/angular/cli/src/commands/mcp/workspace-utils.ts @@ -110,7 +110,7 @@ async function getAllowedWorkspaceRoots(server: McpToolContext['server']): Promi .filter((root): root is string => root !== null); } -async function isAllowedWorkspacePath( +export async function isAllowedWorkspacePath( server: McpToolContext['server'], workspacePath: string, ): Promise { From 5c054c02b35b48b9196034d2fafe357d103ba2c6 Mon Sep 17 00:00:00 2001 From: Herdiyan Adam Putra Date: Sat, 25 Jul 2026 00:08:14 +0700 Subject: [PATCH 2/2] fix(@angular/cli): handle errors from isAllowedWorkspacePath in best-practices tool isAllowedWorkspacePath can throw if the workspace path does not exist (realpathSync) or if listRoots() fails against the connected client. Wrap the check in a try-catch and fall back to the bundled guide on any failure, matching this function's existing fallback behavior. --- .../src/commands/mcp/tools/best-practices.ts | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/angular/cli/src/commands/mcp/tools/best-practices.ts b/packages/angular/cli/src/commands/mcp/tools/best-practices.ts index 3733a9c61641..51fe43c4e648 100644 --- a/packages/angular/cli/src/commands/mcp/tools/best-practices.ts +++ b/packages/angular/cli/src/commands/mcp/tools/best-practices.ts @@ -96,13 +96,24 @@ async function getVersionSpecificBestPractices( logger: McpToolContext['logger'], server: McpToolContext['server'], ): Promise<{ content: string; source: string } | undefined> { - if (server && !(await isAllowedWorkspacePath(server, workspacePath))) { - logger.warn( - `Workspace path is outside the allowed MCP roots: ${workspacePath}. ` + - 'Falling back to the bundled guide.', - ); + if (server) { + try { + if (!(await isAllowedWorkspacePath(server, workspacePath))) { + logger.warn( + `Workspace path is outside the allowed MCP roots: ${workspacePath}. ` + + 'Falling back to the bundled guide.', + ); - return undefined; + return undefined; + } + } catch (e) { + logger.warn( + `Failed to verify workspace path '${workspacePath}': ` + + `${e instanceof Error ? e.message : e}. Falling back to the bundled guide.`, + ); + + return undefined; + } } // 1. Resolve the path to package.json