Skip to content

Commit a71bb31

Browse files
committed
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.
1 parent 9c282d3 commit a71bb31

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

packages/angular/cli/src/commands/mcp/tools/best-practices.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { createRequire } from 'node:module';
2020
import { dirname, isAbsolute, join, relative, resolve } from 'node:path';
2121
import { z } from 'zod';
2222
import { VERSION } from '../../../utilities/version';
23+
import { isAllowedWorkspacePath } from '../workspace-utils';
2324
import { type McpToolContext, declareTool } from './tool-registry';
2425

2526
const bestPracticesInputSchema = z.object({
@@ -86,13 +87,24 @@ async function getBundledBestPractices(): Promise<string> {
8687
*
8788
* @param workspacePath The absolute path to the user's `angular.json` file.
8889
* @param logger The MCP tool context logger for reporting warnings.
90+
* @param server The MCP server context, used to enforce the client's declared roots.
8991
* @returns A promise that resolves to an object containing the guide's content and source,
9092
* or `undefined` if the guide could not be resolved.
9193
*/
9294
async function getVersionSpecificBestPractices(
9395
workspacePath: string,
9496
logger: McpToolContext['logger'],
97+
server: McpToolContext['server'],
9598
): Promise<{ content: string; source: string } | undefined> {
99+
if (server && !(await isAllowedWorkspacePath(server, workspacePath))) {
100+
logger.warn(
101+
`Workspace path is outside the allowed MCP roots: ${workspacePath}. ` +
102+
'Falling back to the bundled guide.',
103+
);
104+
105+
return undefined;
106+
}
107+
96108
// 1. Resolve the path to package.json
97109
let pkgJsonPath: string;
98110
try {
@@ -175,7 +187,7 @@ async function getVersionSpecificBestPractices(
175187
* @param context The MCP tool context, containing the logger.
176188
* @returns An async function that serves as the tool's executor.
177189
*/
178-
function createBestPracticesHandler({ logger }: McpToolContext) {
190+
function createBestPracticesHandler({ logger, server }: McpToolContext) {
179191
let bundledBestPractices: Promise<string>;
180192

181193
return async (input: BestPracticesInput) => {
@@ -184,7 +196,11 @@ function createBestPracticesHandler({ logger }: McpToolContext) {
184196

185197
// First, try to get the version-specific guide.
186198
if (input.workspacePath) {
187-
const versionSpecific = await getVersionSpecificBestPractices(input.workspacePath, logger);
199+
const versionSpecific = await getVersionSpecificBestPractices(
200+
input.workspacePath,
201+
logger,
202+
server,
203+
);
188204
if (versionSpecific) {
189205
content = versionSpecific.content;
190206
source = versionSpecific.source;

packages/angular/cli/src/commands/mcp/workspace-utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ async function getAllowedWorkspaceRoots(server: McpToolContext['server']): Promi
110110
.filter((root): root is string => root !== null);
111111
}
112112

113-
async function isAllowedWorkspacePath(
113+
export async function isAllowedWorkspacePath(
114114
server: McpToolContext['server'],
115115
workspacePath: string,
116116
): Promise<boolean> {

0 commit comments

Comments
 (0)