Ensure devtoolsOptionsUri points to a served extension

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -97,6 +97,10 @@ TODO: Remove this section if there are not any updates.
   the connected app's package root is registered with the Dart Tooling Daemon
   before available DevTools extensions are refreshed.
   [#10086](https://github.com/flutter/devtools/pull/10086)
+* Confined DevTools extension options file access to currently served
+  extensions, rejected symlinked `devtools_options.yaml` files, and prevented
+  creating missing parent directories.
+  [#10088](https://github.com/flutter/devtools/pull/10088)
 
 ## Advanced developer mode updates
 

packages/devtools_shared/CHANGELOG.md

@@ -28,6 +28,9 @@ found in the LICENSE file or at https://developers.google.com/open-source/licens
   non-directory `buildLocation` paths, and symbolic links resolving outside the
   extension build or package root in `ExtensionsManager`, and store valid
   resolved paths in `DevToolsExtensionConfig.extensionAssetsPath`.
+* Confine `devtoolsOptionsUri` in `api/extensionEnabledState` to currently
+  served extensions, reject symlinked `devtools_options.yaml` files, and prevent
+  recursive directory creation when looking up `devtools_options.yaml`.
 
 # 14.0.0
 

packages/devtools_shared/lib/src/extensions/extension_enablement.dart

@@ -124,15 +124,20 @@ $_extensionsKey:
     )?.writeAsStringSync(yamlEditor.toString(), flush: true);
   }
 
-  /// Returns the `devtools_options.yaml` file at [optionsUri].
+  /// Returns the `devtools_options.yaml` file at [optionsUri], or `null` if its
+  /// parent directory does not exist or the file is a symbolic link.
   ///
   /// If the `devtools_options.yaml` does not already exist, it will be created
   /// and written with [_defaultOptions], and then returned.
   File? _lookupOptionsFile(Uri optionsUri) {
     final optionsFile = File.fromUri(optionsUri);
+    if (!optionsFile.parent.existsSync() ||
+        FileSystemEntity.isLinkSync(optionsFile.path)) {
+      return null;
+    }
     if (!optionsFile.existsSync()) {
       optionsFile
-        ..createSync(recursive: true)
+        ..createSync()
         ..writeAsStringSync(_defaultOptions, flush: true);
     }
     return optionsFile;

packages/devtools_shared/lib/src/server/handlers/_devtools_extensions.dart

@@ -78,6 +78,7 @@ extension _ExtensionsApiHandler on Never {
   static shelf.Response handleExtensionEnabledState(
     ServerApi api,
     Map<String, String> queryParams,
+    ExtensionsManager extensionsManager,
   ) {
     final missingRequiredParams = ServerApi._checkRequiredParameters(
       [
@@ -108,21 +109,56 @@ extension _ExtensionsApiHandler on Never {
     final fileName = isFileUri
         ? p.basename(devtoolsOptionsFileUri.toFilePath())
         : '';
-    if (!isFileUri || fileName != 'devtools_options.yaml') {
+    if (!isFileUri || fileName != devtoolsOptionsFileName) {
       return api.badRequest(
         'Invalid devtoolsOptionsUri: must be a file: URI named '
-        "'devtools_options.yaml'.",
+        "'$devtoolsOptionsFileName'.",
+      );
+    }
+
+    final requestedOptionsPath = _resolveOptionsFilePath(
+      devtoolsOptionsFileUri,
+    );
+
+    // Make sure that this extension's `devtools_options.yaml` file is
+    // being served by [ExtensionsManager].
+    //
+    // This prevents DevTools from creating missing directories on disk when
+    // this path points outside of the extension directory.
+    final isServedExtensionOptionsFile =
+        requestedOptionsPath != null &&
+        extensionsManager.devtoolsExtensions.any((ext) {
+          final uri = Uri.tryParse(ext.devtoolsOptionsUri);
+          if (uri == null) return false;
+          if (uri.scheme == 'file' && uri.host.isEmpty) {
+            // If these are equal, there's no need to call
+            // _resolveOptionsFilePath again, so return early.
+            final servedPathUnresolved = p.normalize(uri.toFilePath());
+            if (p.equals(servedPathUnresolved, requestedOptionsPath)) {
+              return true;
+            }
+          }
+          final servedPath = _resolveOptionsFilePath(uri);
+          return servedPath != null &&
+              p.equals(servedPath, requestedOptionsPath);
+        });
+
+    if (!isServedExtensionOptionsFile) {
+      return api.badRequest(
+        'Invalid devtoolsOptionsUri: must belong to a currently served '
+        'DevTools extension.',
       );
     }
 
+    final optionsFileUri = Uri.file(requestedOptionsPath);
     final extensionName = queryParams[ExtensionsApi.extensionNamePropertyName]!;
     final extensionPackage =
         queryParams[ExtensionsApi.extensionPackagePropertyName];
 
     final activate = queryParams[ExtensionsApi.enabledStatePropertyName];
     if (activate != null) {
       final newState = ServerApi._devToolsOptions.setExtensionEnabledState(
-        devtoolsOptionsUri: devtoolsOptionsFileUri,
+        devtoolsOptionsUri: optionsFileUri,
         extensionName: extensionName,
         packageName: extensionPackage,
         enable: bool.parse(activate),
@@ -131,10 +167,35 @@ extension _ExtensionsApiHandler on Never {
     }
     final activationState = ServerApi._devToolsOptions
         .lookupExtensionEnabledState(
-          devtoolsOptionsUri: devtoolsOptionsFileUri,
+          devtoolsOptionsUri: optionsFileUri,
           extensionName: extensionName,
           packageName: extensionPackage,
         );
     return ServerApi._encodeResponse(activationState.name, api: api);
   }
+
+  /// Resolves symbolic links on the parent directory of [uri] to return the
+  /// absolute path to `devtools_options.yaml`.
+  ///
+  /// Returns the actual file path to `devtools_options.yaml`, or `null` if
+  /// [uri] is not a local `file:` URI, its parent directory does not exist, or
+  /// the `devtools_options.yaml` file itself is a symbolic link.
+  static String? _resolveOptionsFilePath(Uri uri) {
+    if (uri.scheme != 'file' || uri.host.isNotEmpty) return null;
+
+    final filePath = p.normalize(uri.toFilePath());
+    if (p.basename(filePath) != devtoolsOptionsFileName) return null;
+
+    final parentDir = Directory(p.dirname(filePath));
+    if (!parentDir.existsSync()) return null;
+
+    try {
+      final resolvedParent = p.normalize(parentDir.resolveSymbolicLinksSync());
+      final resolvedFilePath = p.join(resolvedParent, devtoolsOptionsFileName);
+      if (FileSystemEntity.isLinkSync(resolvedFilePath)) return null;
+      return resolvedFilePath;
+    } on FileSystemException {
+      return null;
+    }
+  }
 }

packages/devtools_shared/lib/src/server/server_api.dart

@@ -18,6 +18,7 @@ import 'package:vm_service/vm_service.dart';
 import '../common.dart';
 import '../deeplink/deeplink_manager.dart';
 import '../devtools_api.dart';
+import '../extensions/constants.dart';
 import '../extensions/extension_enablement.dart';
 import '../extensions/extension_manager.dart';
 import '../service/service.dart';
@@ -347,6 +348,7 @@ class ServerApi {
         return _ExtensionsApiHandler.handleExtensionEnabledState(
           api,
           queryParams,
+          extensionsManager,
         );
 
       // ----- deeplink api. -----

packages/devtools_shared/test/extensions/extension_enablement_test.dart

@@ -156,5 +156,85 @@ extensions:
   - provider: true
   - custom_pkg: true''');
     });
+
+    test(
+      'returns error and does not create parent directory when parent does not '
+      'exist',
+      () {
+        final missingParentDir = Directory(
+          p.join(tmpDir.path, 'missing_parent'),
+        );
+        final missingOptionsUri = Uri.file(
+          p.join(missingParentDir.path, devtoolsOptionsFileName),
+        );
+        expect(
+          options.lookupExtensionEnabledState(
+            devtoolsOptionsUri: missingOptionsUri,
+            extensionName: 'foo',
+          ),
+          ExtensionEnabledState.error,
+        );
+        expect(
+          options.setExtensionEnabledState(
+            devtoolsOptionsUri: missingOptionsUri,
+            extensionName: 'foo',
+            enable: true,
+          ),
+          ExtensionEnabledState.error,
+        );
+        expect(missingParentDir.existsSync(), isFalse);
+      },
+    );
+
+    test(
+      'returns error and does not follow symlink when devtools_options.yaml is '
+      'a symlink',
+      () {
+        final targetFile = File(p.join(tmpDir.path, 'target.yaml'))
+          ..writeAsStringSync('original');
+        final brokenTargetFile = File(
+          p.join(tmpDir.path, 'broken_target.yaml'),
+        );
+
+        final optionsLink = Link(p.join(tmpDir.path, devtoolsOptionsFileName))
+          ..createSync(targetFile.path);
+        expect(
+          options.lookupExtensionEnabledState(
+            devtoolsOptionsUri: optionsUri,
+            extensionName: 'foo',
+          ),
+          ExtensionEnabledState.error,
+        );
+        expect(
+          options.setExtensionEnabledState(
+            devtoolsOptionsUri: optionsUri,
+            extensionName: 'foo',
+            enable: true,
+          ),
+          ExtensionEnabledState.error,
+        );
+        expect(targetFile.readAsStringSync(), 'original');
+
+        optionsLink
+          ..deleteSync()
+          ..createSync(brokenTargetFile.path);
+        expect(
+          options.lookupExtensionEnabledState(
+            devtoolsOptionsUri: optionsUri,
+            extensionName: 'foo',
+          ),
+          ExtensionEnabledState.error,
+        );
+        expect(
+          options.setExtensionEnabledState(
+            devtoolsOptionsUri: optionsUri,
+            extensionName: 'foo',
+            enable: true,
+          ),
+          ExtensionEnabledState.error,
+        );
+        expect(brokenTargetFile.existsSync(), isFalse);
+      },
+    );
   });
 }

packages/devtools_shared/test/server/devtools_extensions_api_test.dart

@@ -716,6 +716,101 @@ void main() {
       }
     });
 
+    test('rejects devtoolsOptionsUri in a non-existent directory and does not '
+        'create the directory', () async {
+      await serveExtensions(extensionsManager);
+      final nonExistentDir = Directory(
+        p.join(extensionTestManager.testDirectory.path, 'non_existent_dir'),
+      );
+      expect(nonExistentDir.existsSync(), isFalse);
+
+      final invalidOptionsUri = Uri.file(
+        p.join(nonExistentDir.path, devtoolsOptionsFileName),
+      ).toString();
+      final response = await sendEnabledStateRequest(
+        extensionName: 'drift',
+        enable: true,
+        optionsUriOverride: invalidOptionsUri,
+      );
+
+      expect(response.statusCode, HttpStatus.badRequest);
+      expect(nonExistentDir.existsSync(), isFalse);
+    });
+
+    test('rejects devtoolsOptionsUri in an existing directory or unserved Dart '
+        'package directory', () async {
+      await serveExtensions(extensionsManager);
+
+      final nonPackageDir = Directory(
+        p.join(extensionTestManager.testDirectory.path, 'non_package_dir'),
+      )..createSync(recursive: true);
+      final unservedPackageDir = Directory(
+        p.join(extensionTestManager.testDirectory.path, 'unserved_pkg'),
+      )..createSync(recursive: true);
+
+      const unservedPubspec = '''
+name: unserved_pkg
+environment:
+  sdk: "^3.5.0"
+''';
+      File(
+        p.join(unservedPackageDir.path, 'pubspec.yaml'),
+      ).writeAsStringSync(unservedPubspec);
+
+      for (final dir in [nonPackageDir, unservedPackageDir]) {
+        final invalidOptionsFile = File(
+          p.join(dir.path, devtoolsOptionsFileName),
+        );
+        final response = await sendEnabledStateRequest(
+          extensionName: 'drift',
+          enable: true,
+          optionsUriOverride: invalidOptionsFile.uri.toString(),
+        );
+
+        expect(response.statusCode, HttpStatus.badRequest);
+        expect(invalidOptionsFile.existsSync(), isFalse);
+      }
+    });
+
+    test(
+      'rejects devtoolsOptionsUri when devtools_options.yaml is a symlink',
+      () async {
+        await serveExtensions(extensionsManager);
+
+        final outsideTarget = File(
+          p.join(extensionTestManager.testDirectory.path, 'outside_target.txt'),
+        )..writeAsStringSync('unchanged');
+        final brokenTarget = File(
+          p.join(extensionTestManager.testDirectory.path, 'broken_target.txt'),
+        );
+
+        final optionsLink = Link(optionsFile.path)
+          ..createSync(outsideTarget.path);
+        addTearDown(() {
+          if (FileSystemEntity.isLinkSync(optionsLink.path)) {
+            optionsLink.deleteSync();
+          }
+        });
+
+        var response = await sendEnabledStateRequest(
+          extensionName: 'drift',
+          enable: true,
+        );
+        expect(response.statusCode, HttpStatus.badRequest);
+        expect(outsideTarget.readAsStringSync(), 'unchanged');
+
+        optionsLink
+          ..deleteSync()
+          ..createSync(brokenTarget.path);
+        response = await sendEnabledStateRequest(
+          extensionName: 'drift',
+          enable: true,
+        );
+        expect(response.statusCode, HttpStatus.badRequest);
+        expect(brokenTarget.existsSync(), isFalse);
+      },
+    );
+
     test('accepts a valid devtools_options.yaml file: URI', () async {
       await serveExtensions(extensionsManager);
       final response = await sendEnabledStateRequest(extensionName: 'drift');

pubspec.lock

@@ -891,26 +891,26 @@ packages:
     dependency: transitive
     description:
       name: test
-      sha256: de5d145b0afff7921e5e788a880f52d7e5f3ae24068a202f6fd3b58e4ba26323
+      sha256: "44b2fccf4407bcdb5e105c064c5d6b5e2a640c87761f8b03cdce8cf532d98cfa"
       url: "https://pub.dev"
     source: hosted
-    version: "1.32.0"
+    version: "1.33.0"
   test_api:
     dependency: transitive
     description:
       name: test_api
-      sha256: "0a10344e901e5b2e63819567951cb6a06673ed6b84f40462188ff5a0c41f371f"
+      sha256: "47481c58bb92d0b1d218f0510ff5647e768f8e1256d07077a151fb520923b51d"
       url: "https://pub.dev"
     source: hosted
-    version: "0.7.14"
+    version: "0.7.15"
   test_core:
     dependency: transitive
     description:
       name: test_core
-      sha256: "80f3fb49087454e07e7e07c67578cfdd156c8c3a5227d8b3f47c7b2d019c2e93"
+      sha256: "9dec4c178bd61493e1cd2060ffe701fcb61c43da2493e99590fb1b0cc82eb4af"
       url: "https://pub.dev"
     source: hosted
-    version: "0.6.20"
+    version: "0.6.21"
   typed_data:
     dependency: transitive
     description: