Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/devtools_shared/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ Copyright 2025 The Flutter Authors
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
-->
# 13.0.2-wip

# 13.0.2
* Validate the `devtoolsOptionsUri` query parameter in the extension enabled
state handler so it must be a `file:` URI named `devtools_options.yaml`,
preventing arbitrary file writes by the DevTools server process.
* The minimum Dart SDK version is bumped to 3.11.0.
* The minimum Flutter SDK version is bumped to 3.41.0.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,27 @@ extension _ExtensionsApiHandler on Never {
final devtoolsOptionsFileUriString =
queryParams[ExtensionsApi.devtoolsOptionsUriPropertyName]!;
final devtoolsOptionsFileUri = Uri.parse(devtoolsOptionsFileUriString);

// Validate that the URI is a local file URI whose file name is exactly
// 'devtools_options.yaml'. Accepting arbitrary URIs from the query string
// would allow an untrusted caller to create or overwrite any file writable
// by the DevTools server process. Resolving the name through
// `Uri.toFilePath()` + `p.basename` handles both '/' and '\' path
// separators, so the check holds for Windows file URIs as well. Requiring
// an empty host rejects UNC paths (e.g. `file://server/share/...`) and
// keeps `toFilePath()` from throwing on a non-local authority.
final isFileUri = devtoolsOptionsFileUri.scheme == 'file' &&
devtoolsOptionsFileUri.host.isEmpty;
final fileName = isFileUri
? p.basename(devtoolsOptionsFileUri.toFilePath())
: '';
if (!isFileUri || fileName != 'devtools_options.yaml') {
return api.badRequest(
'Invalid devtoolsOptionsUri: must be a file: URI named '
"'devtools_options.yaml'.",
);
}

final extensionName = queryParams[ExtensionsApi.extensionNamePropertyName]!;

final activate = queryParams[ExtensionsApi.enabledStatePropertyName];
Expand Down
1 change: 1 addition & 0 deletions packages/devtools_shared/lib/src/server/server_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'dart:io';
import 'package:collection/collection.dart';
import 'package:dtd/dtd.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Necessary any longer?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is necessary for use in the _devtools_extensions.dart part file.

import 'package:shelf/shelf.dart' as shelf;
import 'package:vm_service/vm_service.dart';

Expand Down
2 changes: 1 addition & 1 deletion packages/devtools_shared/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
name: devtools_shared
description: Package of shared Dart structures between devtools_app, dds, and other tools.

version: 13.0.2-dev
version: 13.0.2

repository: https://github.com/flutter/devtools/tree/master/packages/devtools_shared

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ void main() {
includeDependenciesWithExtensions: includeDependenciesWithExtensions,
includeBadExtension: includeBadExtension,
);
await testDtdConnection!.setIDEWorkspaceRoots(
dtd!.info!.secret!,
[extensionTestManager.packagesRootUri],
);
await testDtdConnection!.setIDEWorkspaceRoots(dtd!.info!.secret!, [
extensionTestManager.packagesRootUri,
]);
}

Future<Response> serveExtensions(
Expand All @@ -66,8 +65,9 @@ void main() {
host: 'localhost',
path: ExtensionsApi.apiServeAvailableExtensions,
queryParameters: {
ExtensionsApi.packageRootUriPropertyName:
includeRuntimeRoot ? extensionTestManager.runtimeAppRoot : null,
ExtensionsApi.packageRootUriPropertyName: includeRuntimeRoot
? extensionTestManager.runtimeAppRoot
: null,
},
),
);
Expand Down Expand Up @@ -128,9 +128,7 @@ void main() {
test(
'fails when an exception is thrown and there are no valid extensions',
() async {
await initializeTestDirectory(
includeDependenciesWithExtensions: false,
);
await initializeTestDirectory(includeDependenciesWithExtensions: false);
extensionsManager = _TestExtensionsManager();
final response = await serveExtensions(extensionsManager);
expect(response.statusCode, HttpStatus.internalServerError);
Expand Down Expand Up @@ -159,6 +157,7 @@ void main() {
Future<Response> sendEnabledStateRequest({
required String extensionName,
bool? enable,
String? optionsUriOverride,
}) async {
final request = Request(
'post',
Expand All @@ -167,7 +166,8 @@ void main() {
host: 'localhost',
path: ExtensionsApi.apiExtensionEnabledState,
queryParameters: {
ExtensionsApi.devtoolsOptionsUriPropertyName: optionsFileUriString,
ExtensionsApi.devtoolsOptionsUriPropertyName:
optionsUriOverride ?? optionsFileUriString,
ExtensionsApi.extensionNamePropertyName: extensionName,
if (enable != null)
ExtensionsApi.enabledStatePropertyName: enable.toString(),
Expand All @@ -181,6 +181,37 @@ void main() {
);
}

test('rejects a devtoolsOptionsUri that is not a devtools_options.yaml '
'file', () async {
await serveExtensions(extensionsManager);
const invalidUris = [
// Wrong file name.
'file:///tmp/evil.txt',
'file:///tmp/devtools_options.yaml.bak',
// Non-file scheme.
'https://evil.example.com/devtools_options.yaml',
// UNC path / non-empty host.
'file://remotehost/share/devtools_options.yaml',
];
for (final invalidUri in invalidUris) {
final response = await sendEnabledStateRequest(
extensionName: 'drift',
optionsUriOverride: invalidUri,
);
expect(
response.statusCode,
HttpStatus.badRequest,
reason: 'expected $invalidUri to be rejected',
);
}
});

test('accepts a valid devtools_options.yaml file: URI', () async {
await serveExtensions(extensionsManager);
final response = await sendEnabledStateRequest(extensionName: 'drift');
expect(response.statusCode, HttpStatus.ok);
});

test('options file does not exist until first acesss', () async {
await serveExtensions(extensionsManager);
expect(optionsFile.existsSync(), isFalse);
Expand All @@ -202,16 +233,16 @@ void main() {
ExtensionEnabledState.none.name,
);

// TODO(kenz): why is existsSync() returning false when I can verify the file
// contents on the file system at [optionsFileUriString]?
// expect(optionsFile.existsSync(), isTrue);
// expect(
// optionsFile.readAsStringSync(),
// '''
// description: This file stores settings for Dart & Flutter DevTools.
// documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states
// extensions:''',
// );
// TODO(kenz): why is existsSync() returning false when I can verify the file
// contents on the file system at [optionsFileUriString]?
// expect(optionsFile.existsSync(), isTrue);
// expect(
// optionsFile.readAsStringSync(),
// '''
// description: This file stores settings for Dart & Flutter DevTools.
// documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states
// extensions:''',
// );

response = await sendEnabledStateRequest(
extensionName: 'drift',
Expand All @@ -233,16 +264,16 @@ void main() {
ExtensionEnabledState.disabled.name,
);

// expect(optionsFile.existsSync(), isTrue);
// expect(
// optionsFile.readAsStringSync(),
// '''
// description: This file stores settings for Dart & Flutter DevTools.
// documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states
// extensions:
// - drift: true
// - provider: false''',
// );
// expect(optionsFile.existsSync(), isTrue);
// expect(
// optionsFile.readAsStringSync(),
// '''
// description: This file stores settings for Dart & Flutter DevTools.
// documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states
// extensions:
// - drift: true
// - provider: false''',
// );
});
});
}
Expand Down
Loading