Skip to content

Commit 66c01cc

Browse files
jakemac53jakemac53
authored
Require roots for all CLI tools (#101)
Updates all CLI tools to only allow them to be ran under one of the client specified roots. - If paths are specified, those also must not escape the roots. - Make roots not required any longer, use the client specified roots by default for all tools. - Create some shared constants for argument names and a shared function to create the schemas for the roots parameters.
1 parent f46543a commit 66c01cc

File tree

8 files changed

+298
-121
lines changed

8 files changed

+298
-121
lines changed

‎pkgs/dart_tooling_mcp_server/lib/src/mixins/analyzer.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ base mixin DartAnalyzerSupport
271271
return CallToolResult(content: [TextContent(text: jsonEncode(result))]);
272272
}
273273

274-
/// Ensures that all prerequites for any analysis task are met.
274+
/// Ensures that all prerequisites for any analysis task are met.
275275
///
276276
/// Returns an error response if any prerequisite is not met, otherwise
277277
/// returns `null`.

‎pkgs/dart_tooling_mcp_server/lib/src/mixins/dart_cli.dart‎

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'dart:async';
77
import 'package:dart_mcp/server.dart';
88

99
import '../utils/cli_utils.dart';
10+
import '../utils/constants.dart';
1011
import '../utils/process_manager.dart';
1112

1213
// TODO: migrate the analyze files tool to use this mixin and run the
@@ -17,13 +18,19 @@ import '../utils/process_manager.dart';
1718
///
1819
/// The MCPServer must already have the [ToolsSupport] and [LoggingSupport]
1920
/// mixins applied.
20-
base mixin DartCliSupport on ToolsSupport, LoggingSupport
21+
base mixin DartCliSupport on ToolsSupport, LoggingSupport, RootsTrackingSupport
2122
implements ProcessManagerSupport {
2223
@override
2324
FutureOr<InitializeResult> initialize(InitializeRequest request) {
24-
registerTool(dartFixTool, _runDartFixTool);
25-
registerTool(dartFormatTool, _runDartFormatTool);
26-
return super.initialize(request);
25+
try {
26+
return super.initialize(request);
27+
} finally {
28+
// Can't call this until after `super.initialize`.
29+
if (supportsRoots) {
30+
registerTool(dartFixTool, _runDartFixTool);
31+
registerTool(dartFormatTool, _runDartFormatTool);
32+
}
33+
}
2734
}
2835

2936
/// Implementation of the [dartFixTool].
@@ -33,6 +40,7 @@ base mixin DartCliSupport on ToolsSupport, LoggingSupport
3340
command: ['dart', 'fix', '--apply'],
3441
commandDescription: 'dart fix',
3542
processManager: processManager,
43+
knownRoots: await roots,
3644
);
3745
}
3846

@@ -44,6 +52,7 @@ base mixin DartCliSupport on ToolsSupport, LoggingSupport
4452
commandDescription: 'dart format',
4553
processManager: processManager,
4654
defaultPaths: ['.'],
55+
knownRoots: await roots,
4756
);
4857
}
4958

@@ -52,22 +61,7 @@ base mixin DartCliSupport on ToolsSupport, LoggingSupport
5261
description: 'Runs `dart fix --apply` for the given project roots.',
5362
annotations: ToolAnnotations(title: 'Dart fix', destructiveHint: true),
5463
inputSchema: Schema.object(
55-
properties: {
56-
'roots': Schema.list(
57-
title: 'All projects roots to run dart fix in.',
58-
description:
59-
'These must match a root returned by a call to "listRoots".',
60-
items: Schema.object(
61-
properties: {
62-
'root': Schema.string(
63-
title: 'The URI of the project root to run `dart fix` in.',
64-
),
65-
},
66-
required: ['root'],
67-
),
68-
),
69-
},
70-
required: ['roots'],
64+
properties: {ParameterNames.roots: rootsSchema()},
7165
),
7266
);
7367

@@ -76,29 +70,7 @@ base mixin DartCliSupport on ToolsSupport, LoggingSupport
7670
description: 'Runs `dart format .` for the given project roots.',
7771
annotations: ToolAnnotations(title: 'Dart format', destructiveHint: true),
7872
inputSchema: Schema.object(
79-
properties: {
80-
'roots': Schema.list(
81-
title: 'All projects roots to run dart format in.',
82-
description:
83-
'These must match a root returned by a call to "listRoots".',
84-
items: Schema.object(
85-
properties: {
86-
'root': Schema.string(
87-
title: 'The URI of the project root to run `dart format` in.',
88-
),
89-
'paths': Schema.list(
90-
title:
91-
'Relative or absolute paths to analyze under the '
92-
'"root". Paths must correspond to files and not '
93-
'directories.',
94-
items: Schema.string(),
95-
),
96-
},
97-
required: ['root'],
98-
),
99-
),
100-
},
101-
required: ['roots'],
73+
properties: {ParameterNames.roots: rootsSchema(supportsPaths: true)},
10274
),
10375
);
10476
}

‎pkgs/dart_tooling_mcp_server/lib/src/mixins/pub.dart‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'dart:async';
77
import 'package:dart_mcp/server.dart';
88

99
import '../utils/cli_utils.dart';
10+
import '../utils/constants.dart';
1011
import '../utils/process_manager.dart';
1112

1213
/// Mix this in to any MCPServer to add support for running Pub commands like
@@ -16,17 +17,22 @@ import '../utils/process_manager.dart';
1617
///
1718
/// The MCPServer must already have the [ToolsSupport] and [LoggingSupport]
1819
/// mixins applied.
19-
base mixin PubSupport on ToolsSupport, LoggingSupport
20+
base mixin PubSupport on ToolsSupport, LoggingSupport, RootsTrackingSupport
2021
implements ProcessManagerSupport {
2122
@override
2223
FutureOr<InitializeResult> initialize(InitializeRequest request) {
23-
registerTool(pubTool, _runDartPubTool);
24-
return super.initialize(request);
24+
try {
25+
return super.initialize(request);
26+
} finally {
27+
if (supportsRoots) {
28+
registerTool(pubTool, _runDartPubTool);
29+
}
30+
}
2531
}
2632

2733
/// Implementation of the [pubTool].
2834
Future<CallToolResult> _runDartPubTool(CallToolRequest request) async {
29-
final command = request.arguments?['command'] as String?;
35+
final command = request.arguments?[ParameterNames.command] as String?;
3036
if (command == null) {
3137
return CallToolResult(
3238
content: [TextContent(text: 'Missing required argument `command`.')],
@@ -48,7 +54,8 @@ base mixin PubSupport on ToolsSupport, LoggingSupport
4854
);
4955
}
5056

51-
final packageName = request.arguments?['packageName'] as String?;
57+
final packageName =
58+
request.arguments?[ParameterNames.packageName] as String?;
5259
if (matchingCommand.requiresPackageName && packageName == null) {
5360
return CallToolResult(
5461
content: [
@@ -69,6 +76,7 @@ base mixin PubSupport on ToolsSupport, LoggingSupport
6976
command: ['dart', 'pub', command, if (packageName != null) packageName],
7077
commandDescription: 'dart pub $command',
7178
processManager: processManager,
79+
knownRoots: await roots,
7280
);
7381
}
7482

@@ -80,34 +88,20 @@ base mixin PubSupport on ToolsSupport, LoggingSupport
8088
annotations: ToolAnnotations(title: 'pub', readOnlyHint: false),
8189
inputSchema: Schema.object(
8290
properties: {
83-
'command': Schema.string(
91+
ParameterNames.command: Schema.string(
8492
title: 'The dart pub command to run.',
8593
description:
8694
'Currently only ${SupportedPubCommand.listAll} are supported.',
8795
),
88-
'packageName': Schema.string(
96+
ParameterNames.packageName: Schema.string(
8997
title: 'The package name to run the command for.',
9098
description:
9199
'This is required for the '
92100
'${SupportedPubCommand.listAllThatRequirePackageName} commands.',
93101
),
94-
'roots': Schema.list(
95-
title: 'All projects roots to run the dart pub command in.',
96-
description:
97-
'These must match a root returned by a call to "listRoots".',
98-
items: Schema.object(
99-
properties: {
100-
'root': Schema.string(
101-
title:
102-
'The URI of the project root to run the dart pub command '
103-
'in.',
104-
),
105-
},
106-
required: ['root'],
107-
),
108-
),
102+
ParameterNames.roots: rootsSchema(),
109103
},
110-
required: ['command', 'roots'],
104+
required: [ParameterNames.command],
111105
),
112106
);
113107
}

‎pkgs/dart_tooling_mcp_server/lib/src/utils/cli_utils.dart‎

Lines changed: 85 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
import 'dart:io';
66

77
import 'package:dart_mcp/server.dart';
8+
import 'package:path/path.dart' as p;
89
import 'package:process/process.dart';
910

11+
import 'constants.dart';
12+
1013
/// Runs [command] in each of the project roots specified in the [request].
1114
///
1215
/// The [command] should be a list of strings that can be passed directly to
@@ -16,6 +19,10 @@ import 'package:process/process.dart';
1619
/// being run. For example, if the command is `['dart', 'fix', '--apply']`, the
1720
/// command description might be `dart fix`.
1821
///
22+
/// The [knownRoots] are used by default if no roots are provided as an
23+
/// argument on the [request]. Otherwise, all roots provided in the request
24+
/// arguments must still be encapsulated by the [knownRoots].
25+
///
1926
/// [defaultPaths] may be specified if one or more path arguments are required
2027
/// for the command (e.g. `dart format <default paths>`). The paths can be
2128
/// absolute or relative paths that point to the directories on which the
@@ -29,20 +36,23 @@ Future<CallToolResult> runCommandInRoots(
2936
required List<String> command,
3037
required String commandDescription,
3138
required ProcessManager processManager,
39+
required List<Root> knownRoots,
3240
List<String> defaultPaths = const <String>[],
3341
}) async {
34-
final rootConfigs =
35-
(request.arguments?['roots'] as List?)?.cast<Map<String, Object?>>();
36-
if (rootConfigs == null) {
37-
return CallToolResult(
38-
content: [TextContent(text: 'Missing required argument `roots`.')],
39-
isError: true,
40-
);
42+
var rootConfigs =
43+
(request.arguments?[ParameterNames.roots] as List?)
44+
?.cast<Map<String, Object?>>();
45+
46+
// Default to use the known roots if none were specified.
47+
if (rootConfigs == null || rootConfigs.isEmpty) {
48+
rootConfigs = [
49+
for (final root in knownRoots) {ParameterNames.root: root.uri},
50+
];
4151
}
4252

4353
final outputs = <TextContent>[];
4454
for (var rootConfig in rootConfigs) {
45-
final rootUriString = rootConfig['root'] as String?;
55+
final rootUriString = rootConfig[ParameterNames.root] as String?;
4656
if (rootUriString == null) {
4757
// This shouldn't happen based on the schema, but handle defensively.
4858
return CallToolResult(
@@ -53,6 +63,19 @@ Future<CallToolResult> runCommandInRoots(
5363
);
5464
}
5565

66+
if (!_isAllowedRoot(rootUriString, knownRoots)) {
67+
return CallToolResult(
68+
content: [
69+
TextContent(
70+
text:
71+
'Invalid root $rootUriString, must be under one of the '
72+
'registered project roots:\n\n${knownRoots.join('\n')}',
73+
),
74+
],
75+
isError: true,
76+
);
77+
}
78+
5679
final rootUri = Uri.parse(rootUriString);
5780
if (rootUri.scheme != 'file') {
5881
return CallToolResult(
@@ -68,9 +91,28 @@ Future<CallToolResult> runCommandInRoots(
6891
}
6992
final projectRoot = Directory(rootUri.toFilePath());
7093

71-
final commandWithPaths = List<String>.from(command);
72-
final paths = (rootConfig['paths'] as List?)?.cast<String>();
73-
commandWithPaths.addAll(paths ?? defaultPaths);
94+
final commandWithPaths = List.of(command);
95+
final paths =
96+
(rootConfig[ParameterNames.paths] as List?)?.cast<String>() ??
97+
defaultPaths;
98+
final invalidPaths = paths.where((path) {
99+
final resolvedPath = rootUri.resolve(path).toString();
100+
return rootUriString != resolvedPath &&
101+
!p.isWithin(rootUriString, resolvedPath);
102+
});
103+
if (invalidPaths.isNotEmpty) {
104+
return CallToolResult(
105+
content: [
106+
TextContent(
107+
text:
108+
'Paths are not allowed to escape their project root:\n'
109+
'${invalidPaths.join('\n')}',
110+
),
111+
],
112+
isError: true,
113+
);
114+
}
115+
commandWithPaths.addAll(paths);
74116

75117
final result = await processManager.run(
76118
commandWithPaths,
@@ -102,3 +144,35 @@ Future<CallToolResult> runCommandInRoots(
102144
}
103145
return CallToolResult(content: outputs);
104146
}
147+
148+
/// Returns whether or not [rootUri] is an allowed root, either exactly matching
149+
/// or under on of the [knownRoots].
150+
bool _isAllowedRoot(String rootUri, List<Root> knownRoots) =>
151+
knownRoots.any((knownRoot) {
152+
final knownRootUri = Uri.parse(knownRoot.uri);
153+
final resolvedRoot = knownRootUri.resolve(rootUri).toString();
154+
return knownRoot.uri == resolvedRoot ||
155+
p.isWithin(knownRoot.uri, resolvedRoot);
156+
});
157+
158+
/// The schema for the `roots` parameter for any tool that accepts it.
159+
ListSchema rootsSchema({bool supportsPaths = false}) => Schema.list(
160+
title: 'All projects roots to run this tool in.',
161+
items: Schema.object(
162+
properties: {
163+
ParameterNames.root: Schema.string(
164+
title: 'The URI of the project root to run this tool in.',
165+
description:
166+
'This must be equal to or a subdirectory of one of the roots '
167+
'returned by a call to "listRoots".',
168+
),
169+
if (supportsPaths)
170+
ParameterNames.paths: Schema.list(
171+
title:
172+
'Paths to run this tool on. Must resolve to a path that is '
173+
'within the "root".',
174+
),
175+
},
176+
required: [ParameterNames.root],
177+
),
178+
);

‎pkgs/dart_tooling_mcp_server/lib/src/utils/constants.dart‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
/// A namespace for all the parameter names.
6+
extension ParameterNames on Never {
7+
static const command = 'command';
8+
static const packageName = 'packageName';
9+
static const paths = 'paths';
10+
static const root = 'root';
11+
static const roots = 'roots';
12+
}

0 commit comments

Comments
 (0)