Skip to content

Commit 1d9d60c

Browse files
authored
handle relative paths under roots without trailing slashes (#152)
This resolves an issue I was seeing where if an LLM sets a root without a trailing slash, then tools which accept paths would think the path was escaping the root, because Uri.parse('/foo/bar').resolve('zap') results in a uri with the path /foo/zap).
1 parent 6a71aeb commit 1d9d60c

File tree

3 files changed

+64
-50
lines changed

3 files changed

+64
-50
lines changed

.github/workflows/dart_mcp_server.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,6 @@ jobs:
4242
channel: ${{ matrix.flutterSdk }}
4343
cache: true
4444
cache-key: "flutter-:os:-:channel:-:version:-:arch:-:hash:"
45-
# Exposes the DART_SDK environment variable to all other actions.
46-
# https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#setting-an-environment-variable
47-
- run: echo "DART_SDK=$(which dart | xargs dirname | xargs dirname)" >> "$GITHUB_ENV"
4845

4946
- name: fetch counter app deps
5047
working-directory: pkgs/dart_mcp_server/test_fixtures/counter_app

pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import 'dart:async';
77
import 'package:collection/collection.dart';
88
import 'package:dart_mcp/server.dart';
99
import 'package:file/file.dart';
10-
import 'package:path/path.dart' as p;
1110
import 'package:process/process.dart';
1211
import 'package:yaml/yaml.dart';
1312

@@ -164,7 +163,7 @@ Future<CallToolResult> runCommandInRoot(
164163
}
165164

166165
final root = knownRoots.firstWhereOrNull(
167-
(root) => _isUnderRoot(root, rootUriString),
166+
(root) => _isUnderRoot(root, rootUriString, fileSystem),
168167
);
169168
if (root == null) {
170169
return CallToolResult(
@@ -201,7 +200,9 @@ Future<CallToolResult> runCommandInRoot(
201200
final paths =
202201
(rootConfig?[ParameterNames.paths] as List?)?.cast<String>() ??
203202
defaultPaths;
204-
final invalidPaths = paths.where((path) => !_isUnderRoot(root, path));
203+
final invalidPaths = paths.where(
204+
(path) => !_isUnderRoot(root, path, fileSystem),
205+
);
205206
if (invalidPaths.isNotEmpty) {
206207
return CallToolResult(
207208
content: [
@@ -268,8 +269,10 @@ Future<String> defaultCommandForRoot(
268269
/// Returns whether [uri] is under or exactly equal to [root].
269270
///
270271
/// Relative uris will always be under [root] unless they escape it with `../`.
271-
bool _isUnderRoot(Root root, String uri) {
272-
final rootUri = Uri.parse(root.uri);
272+
bool _isUnderRoot(Root root, String uri, FileSystem fileSystem) {
273+
// This normalizes the URI to ensure it is treated as a directory (for example
274+
// ensures it ends with a trailing slash).
275+
final rootUri = fileSystem.directory(Uri.parse(root.uri)).uri;
273276
final resolvedUri = rootUri.resolve(uri);
274277
// We don't care about queries or fragments, but the scheme/authority must
275278
// match.
@@ -279,10 +282,11 @@ bool _isUnderRoot(Root root, String uri) {
279282
}
280283
// Canonicalizing the paths handles any `../` segments and also deals with
281284
// trailing slashes versus no trailing slashes.
282-
final canonicalRootPath = p.canonicalize(rootUri.path);
283-
final canonicalUriPath = p.canonicalize(resolvedUri.path);
285+
286+
final canonicalRootPath = fileSystem.path.canonicalize(rootUri.path);
287+
final canonicalUriPath = fileSystem.path.canonicalize(resolvedUri.path);
284288
return canonicalRootPath == canonicalUriPath ||
285-
canonicalUriPath.startsWith(canonicalRootPath);
289+
fileSystem.path.isWithin(canonicalRootPath, canonicalUriPath);
286290
}
287291

288292
/// The schema for the `roots` parameter for any tool that accepts it.

pkgs/dart_mcp_server/test/utils/cli_utils_test.dart

Lines changed: 52 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -139,45 +139,58 @@ void main() {
139139
});
140140

141141
test('with paths inside of known roots', () async {
142-
final paths = ['file:///foo/', 'file:///foo', './', '.'];
143-
final result = await runCommandInRoots(
144-
CallToolRequest(
145-
name: 'foo',
146-
arguments: {
147-
ParameterNames.roots: [
148-
{ParameterNames.root: 'file:///foo', ParameterNames.paths: paths},
149-
{
150-
ParameterNames.root: 'file:///foo/',
151-
ParameterNames.paths: paths,
152-
},
153-
],
154-
},
155-
),
156-
commandForRoot: (_, _, _) => 'fake',
157-
commandDescription: '',
158-
processManager: processManager,
159-
knownRoots: [Root(uri: 'file:///foo/')],
160-
fileSystem: fileSystem,
161-
sdk: Sdk(),
162-
);
163-
expect(
164-
result.isError,
165-
isNot(true),
166-
reason: result.content.map((c) => (c as TextContent).text).join('\n'),
167-
);
168-
expect(
169-
processManager.commandsRan,
170-
unorderedEquals([
171-
equalsCommand((
172-
command: ['fake', ...paths],
173-
workingDirectory: '/foo/',
174-
)),
175-
equalsCommand((
176-
command: ['fake', ...paths],
177-
workingDirectory: '/foo',
178-
)),
179-
]),
180-
);
142+
// Check with registered roots that do and do not have trailing slashes.
143+
for (final knownRoot in ['file:///foo', 'file:///foo/']) {
144+
processManager.reset();
145+
final paths = [
146+
'file:///foo/',
147+
'file:///foo',
148+
'./',
149+
'.',
150+
'lib/foo.dart',
151+
];
152+
final result = await runCommandInRoots(
153+
CallToolRequest(
154+
name: 'foo',
155+
arguments: {
156+
ParameterNames.roots: [
157+
{
158+
ParameterNames.root: 'file:///foo',
159+
ParameterNames.paths: paths,
160+
},
161+
{
162+
ParameterNames.root: 'file:///foo/',
163+
ParameterNames.paths: paths,
164+
},
165+
],
166+
},
167+
),
168+
commandForRoot: (_, _, _) => 'fake',
169+
commandDescription: '',
170+
processManager: processManager,
171+
knownRoots: [Root(uri: knownRoot)],
172+
fileSystem: fileSystem,
173+
sdk: Sdk(),
174+
);
175+
expect(
176+
result.isError,
177+
isNot(true),
178+
reason: result.content.map((c) => (c as TextContent).text).join('\n'),
179+
);
180+
expect(
181+
processManager.commandsRan,
182+
unorderedEquals([
183+
equalsCommand((
184+
command: ['fake', ...paths],
185+
workingDirectory: '/foo/',
186+
)),
187+
equalsCommand((
188+
command: ['fake', ...paths],
189+
workingDirectory: '/foo',
190+
)),
191+
]),
192+
);
193+
}
181194
});
182195
});
183196

0 commit comments

Comments
 (0)