Be more flexible about roots by comparing canonicalized paths (#148)
Instead of using isWithin and also checking for exact equality, we compare the scheme/authority, and then compare the canonicalized paths.
diff --git a/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart b/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart
index bfacc6c..7ae8ae0 100644
--- a/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart
+++ b/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart
@@ -4,6 +4,7 @@
import 'dart:async';
+import 'package:collection/collection.dart';
import 'package:dart_mcp/server.dart';
import 'package:file/file.dart';
import 'package:path/path.dart' as p;
@@ -159,7 +160,9 @@
);
}
- final root = _findRoot(rootUriString, knownRoots);
+ final root = knownRoots.firstWhereOrNull(
+ (root) => _isUnderRoot(root, rootUriString),
+ );
if (root == null) {
return CallToolResult(
content: [
@@ -195,11 +198,7 @@
final paths =
(rootConfig?[ParameterNames.paths] as List?)?.cast<String>() ??
defaultPaths;
- final invalidPaths = paths.where((path) {
- final resolvedPath = rootUri.resolve(path).toString();
- return rootUriString != resolvedPath &&
- !p.isWithin(rootUriString, resolvedPath);
- });
+ final invalidPaths = paths.where((path) => !_isUnderRoot(root, path));
if (invalidPaths.isNotEmpty) {
return CallToolResult(
content: [
@@ -263,17 +262,24 @@
),
};
-/// Returns whether or not [rootUri] is an allowed root, either exactly matching
-/// or under on of the [knownRoots].
-Root? _findRoot(String rootUri, List<Root> knownRoots) {
- for (final root in knownRoots) {
- final knownRootUri = Uri.parse(root.uri);
- final resolvedRoot = knownRootUri.resolve(rootUri).toString();
- if (root.uri == resolvedRoot || p.isWithin(root.uri, resolvedRoot)) {
- return root;
- }
+/// Returns whether [uri] is under or exactly equal to [root].
+///
+/// Relative uris will always be under [root] unless they escape it with `../`.
+bool _isUnderRoot(Root root, String uri) {
+ final rootUri = Uri.parse(root.uri);
+ final resolvedUri = rootUri.resolve(uri);
+ // We don't care about queries or fragments, but the scheme/authority must
+ // match.
+ if (rootUri.scheme != resolvedUri.scheme ||
+ rootUri.authority != resolvedUri.authority) {
+ return false;
}
- return null;
+ // Canonicalizing the paths handles any `../` segments and also deals with
+ // trailing slashes versus no trailing slashes.
+ final canonicalRootPath = p.canonicalize(rootUri.path);
+ final canonicalUriPath = p.canonicalize(resolvedUri.path);
+ return canonicalRootPath == canonicalUriPath ||
+ canonicalUriPath.startsWith(canonicalRootPath);
}
/// The schema for the `roots` parameter for any tool that accepts it.
diff --git a/pkgs/dart_mcp_server/pubspec.yaml b/pkgs/dart_mcp_server/pubspec.yaml
index 5b257e4..d27c0e1 100644
--- a/pkgs/dart_mcp_server/pubspec.yaml
+++ b/pkgs/dart_mcp_server/pubspec.yaml
@@ -15,6 +15,7 @@
dependencies:
args: ^2.7.0
async: ^2.13.0
+ collection: ^1.19.1
dart_mcp: ^0.2.1
dds_service_extensions: ^2.0.1
devtools_shared: ^11.2.0
diff --git a/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart b/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart
index ab93e26..075f1d8 100644
--- a/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart
+++ b/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart
@@ -83,6 +83,102 @@
expect(fileSystem.directory('/bar/baz').existsSync(), true);
},
);
+
+ test('can run commands with missing trailing slashes for roots', () async {
+ final result = await runCommandInRoots(
+ CallToolRequest(
+ name: 'foo',
+ arguments: {
+ ParameterNames.roots: [
+ {ParameterNames.root: 'file:///bar'},
+ ],
+ },
+ ),
+ commandForRoot: (_, _, _) => 'testCommand',
+ arguments: ['a', 'b'],
+ commandDescription: '',
+ processManager: processManager,
+ knownRoots: [Root(uri: 'file:///bar/')],
+ fileSystem: fileSystem,
+ sdk: Sdk(),
+ );
+ expect(result.isError, isNot(true));
+ expect(processManager.commandsRan, [
+ equalsCommand((
+ command: ['testCommand', 'a', 'b'],
+ workingDirectory: '/bar',
+ )),
+ ]);
+ });
+
+ test('can run commands with extra trailing slashes for roots', () async {
+ final result = await runCommandInRoots(
+ CallToolRequest(
+ name: 'foo',
+ arguments: {
+ ParameterNames.roots: [
+ {ParameterNames.root: 'file:///bar/'},
+ ],
+ },
+ ),
+ commandForRoot: (_, _, _) => 'testCommand',
+ arguments: ['a', 'b'],
+ commandDescription: '',
+ processManager: processManager,
+ knownRoots: [Root(uri: 'file:///bar')],
+ fileSystem: fileSystem,
+ sdk: Sdk(),
+ );
+ expect(result.isError, isNot(true));
+ expect(processManager.commandsRan, [
+ equalsCommand((
+ command: ['testCommand', 'a', 'b'],
+ workingDirectory: '/bar/',
+ )),
+ ]);
+ });
+
+ test('with paths inside of known roots', () async {
+ final paths = ['file:///foo/', 'file:///foo', './', '.'];
+ final result = await runCommandInRoots(
+ CallToolRequest(
+ name: 'foo',
+ arguments: {
+ ParameterNames.roots: [
+ {ParameterNames.root: 'file:///foo', ParameterNames.paths: paths},
+ {
+ ParameterNames.root: 'file:///foo/',
+ ParameterNames.paths: paths,
+ },
+ ],
+ },
+ ),
+ commandForRoot: (_, _, _) => 'fake',
+ commandDescription: '',
+ processManager: processManager,
+ knownRoots: [Root(uri: 'file:///foo/')],
+ fileSystem: fileSystem,
+ sdk: Sdk(),
+ );
+ expect(
+ result.isError,
+ isNot(true),
+ reason: result.content.map((c) => (c as TextContent).text).join('\n'),
+ );
+ expect(
+ processManager.commandsRan,
+ unorderedEquals([
+ equalsCommand((
+ command: ['fake', ...paths],
+ workingDirectory: '/foo/',
+ )),
+ equalsCommand((
+ command: ['fake', ...paths],
+ workingDirectory: '/foo',
+ )),
+ ]),
+ );
+ });
});
group('cannot run commands', () {
