diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 0d7aeb830ce..88973108270 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -79,6 +79,9 @@ TODO: Remove this section if there are not any updates. * Hide the DevTools extensions menu button in single-screen embedded mode (`EmbedMode.embedOne`) on standard screens. [#8507](https://github.com/flutter/devtools/issues/8507) +* Added validation during extension discovery to ensure that an extension's + declared name in `config.yaml` matches the containing package name. + [#9965](https://github.com/flutter/devtools/pull/9965) ## Advanced developer mode updates diff --git a/packages/devtools_app_shared/CHANGELOG.md b/packages/devtools_app_shared/CHANGELOG.md index 7dadf83d6ea..11bbf51bc00 100644 --- a/packages/devtools_app_shared/CHANGELOG.md +++ b/packages/devtools_app_shared/CHANGELOG.md @@ -9,6 +9,7 @@ found in the LICENSE file or at https://developers.google.com/open-source/licens * Fix garbage collection issues with the result list in `asyncEval` on both native VM and web. * The minimum Dart SDK version is bumped to 3.11.0. * The minimum Flutter SDK version is bumped to 3.41.0. +* Updates `devtools_shared` constraint to `^14.0.1`. ## 0.5.1 * Add DevTools-styled text field `DevToolsTextField`. diff --git a/packages/devtools_app_shared/pubspec.yaml b/packages/devtools_app_shared/pubspec.yaml index 8afa9443e83..706aa2c1f64 100644 --- a/packages/devtools_app_shared/pubspec.yaml +++ b/packages/devtools_app_shared/pubspec.yaml @@ -15,7 +15,7 @@ resolution: workspace dependencies: collection: ^1.15.0 dds_service_extensions: ^2.0.0 - devtools_shared: ^14.0.0 + devtools_shared: ^14.0.1 dtd: ^4.0.0 flutter: sdk: flutter diff --git a/packages/devtools_extensions/CHANGELOG.md b/packages/devtools_extensions/CHANGELOG.md index 2d412aec0ac..feb956d9169 100644 --- a/packages/devtools_extensions/CHANGELOG.md +++ b/packages/devtools_extensions/CHANGELOG.md @@ -6,6 +6,9 @@ found in the LICENSE file or at https://developers.google.com/open-source/licens ## 0.5.2-wip * The minimum Dart SDK version is bumped to 3.11.0. * The minimum Flutter SDK version is bumped to 3.41.0. +* Update `devtools_extensions validate` to check that the extension's declared + name in `config.yaml` matches the package name in `pubspec.yaml`. +* Updates `devtools_shared` constraint to `^14.0.1`. ## 0.5.1 * Updates `devtools_app_shared` constraint to `^0.5.1`. diff --git a/packages/devtools_extensions/bin/_validate.dart b/packages/devtools_extensions/bin/_validate.dart index a0542f3c6e7..cc605387c0f 100644 --- a/packages/devtools_extensions/bin/_validate.dart +++ b/packages/devtools_extensions/bin/_validate.dart @@ -78,6 +78,14 @@ void _validateDirectoryContents(String packagePath) { throw FileSystemException('${packageDirectory.path} directory not found'); } + final pubspecFile = File(path.join(packageDirectory.path, 'pubspec.yaml')); + if (!pubspecFile.existsSync()) { + throw const FileSystemException(''' +A pubspec.yaml file is required, but none was found. +See ${ValidateExtensionCommand.docUrl}. +'''); + } + final devtoolsExtensionDir = Directory( path.join(packageDirectory.path, 'extension', 'devtools'), ); @@ -109,6 +117,22 @@ An extension/devtools/config.yaml file is required, but none was found. See ${ValidateExtensionCommand.docUrl}. '''); } + + // Ensure the extension's name matches the package name in pubspec.yaml. + final pubspecYaml = loadYaml(pubspecFile.readAsStringSync()); + if (pubspecYaml is YamlMap) { + final pubspecName = pubspecYaml['name'] as String?; + final configYaml = _configAsMap(packagePath); + final configName = configYaml['name'] as String?; + if (configName != null && + pubspecName != null && + configName != pubspecName) { + throw StateError( + 'The "name" field in config.yaml ($configName) does not match the ' + 'package name in pubspec.yaml ($pubspecName).', + ); + } + } } Map _configAsMap(String packagePath) { diff --git a/packages/devtools_extensions/pubspec.yaml b/packages/devtools_extensions/pubspec.yaml index df7ce763e48..4639c9cbcc4 100644 --- a/packages/devtools_extensions/pubspec.yaml +++ b/packages/devtools_extensions/pubspec.yaml @@ -18,7 +18,7 @@ executables: dependencies: args: ^2.4.2 - devtools_shared: ^14.0.0 + devtools_shared: ^14.0.1 devtools_app_shared: ^0.5.1 flutter: sdk: flutter diff --git a/packages/devtools_extensions/test/validate_test.dart b/packages/devtools_extensions/test/validate_test.dart index 8efcb5e0849..8f287b59106 100644 --- a/packages/devtools_extensions/test/validate_test.dart +++ b/packages/devtools_extensions/test/validate_test.dart @@ -53,4 +53,45 @@ void main() { }); } }); + + group('devtools_extensions validate command fails', () { + test('when config.yaml name does not match pubspec.yaml name', () async { + final tempDir = Directory.systemTemp.createTempSync(); + try { + final extDir = Directory(p.join(tempDir.path, 'extension', 'devtools')) + ..createSync(recursive: true); + Directory(p.join(extDir.path, 'build')).createSync(recursive: true); + File(p.join(extDir.path, 'build', 'index.html')).writeAsStringSync(''); + File(p.join(extDir.path, 'config.yaml')).writeAsStringSync(''' +name: mismatched_name +issueTracker: https://www.google.com/ +version: 1.0.0 +materialIconCodePoint: "0xe50a" +'''); + File(p.join(tempDir.path, 'pubspec.yaml')).writeAsStringSync(''' +name: actual_package_name +environment: + sdk: ^3.2.0 +'''); + + final process = await Process.run('dart', [ + 'run', + 'devtools_extensions', + 'validate', + '-p', + tempDir.path, + ]); + expect( + process.stderr, + contains( + 'Validation error: The "name" field in config.yaml ' + '(mismatched_name) does not match the package name in ' + 'pubspec.yaml (actual_package_name).', + ), + ); + } finally { + tempDir.deleteSync(recursive: true); + } + }); + }); } diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md index 132ea2a0590..9a6e5558b76 100644 --- a/packages/devtools_shared/CHANGELOG.md +++ b/packages/devtools_shared/CHANGELOG.md @@ -1,8 +1,12 @@ +# 14.0.1 + +* Validate that an extension's declared name in `config.yaml` matches the package + name in `package_config.json`. + # 14.0.0 * **Breaking changes**: `LocalFileSystem`, an extension which provided some handy diff --git a/packages/devtools_shared/lib/src/extensions/extension_manager.dart b/packages/devtools_shared/lib/src/extensions/extension_manager.dart index 92ad6016d22..adefbd0997b 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_manager.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_manager.dart @@ -148,6 +148,16 @@ class ExtensionsManager { for (final extension in extensions) { final config = extension.config; + final extensionName = config['name']; + if (extensionName != extension.package) { + parsingErrors.writeln( + 'Ignoring extension from package "${extension.package}": its ' + 'config.yaml declares name "$extensionName", which does not match ' + 'the package name.', + ); + continue; + } + // TODO(https://github.com/dart-lang/pub/issues/4042): make this check // more robust. final isPubliclyHosted = diff --git a/packages/devtools_shared/pubspec.yaml b/packages/devtools_shared/pubspec.yaml index ff121888ba3..4507c5e3cfc 100644 --- a/packages/devtools_shared/pubspec.yaml +++ b/packages/devtools_shared/pubspec.yaml @@ -4,7 +4,7 @@ name: devtools_shared description: Package of shared Dart structures between devtools_app, dds, and other tools. -version: 14.0.0 +version: 14.0.1 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_shared diff --git a/packages/devtools_shared/test/helpers/extension_test_manager.dart b/packages/devtools_shared/test/helpers/extension_test_manager.dart index 323ab819c83..54a0dba9d31 100644 --- a/packages/devtools_shared/test/helpers/extension_test_manager.dart +++ b/packages/devtools_shared/test/helpers/extension_test_manager.dart @@ -82,14 +82,19 @@ class ExtensionTestManager { Future setupTestDirectoryStructure({ bool includeDependenciesWithExtensions = true, bool includeBadExtension = false, + bool includeSpoofedExtension = false, }) async { _testDirectory = Directory.systemTemp.createTempSync(); _setupPackages( includeDependenciesWithExtensions: includeDependenciesWithExtensions, includeBadExtension: includeBadExtension, + includeSpoofedExtension: includeSpoofedExtension, + ); + _setupExtensions( + includeBadExtension: includeBadExtension, + includeSpoofedExtension: includeSpoofedExtension, ); - _setupExtensions(includeBadExtension: includeBadExtension); // Generate the .dart_tool/package_config.json file for each Dart package. final testDirectoryContents = testDirectory.listSync(); @@ -144,10 +149,19 @@ class ExtensionTestManager { void _setupPackages({ required bool includeDependenciesWithExtensions, required bool includeBadExtension, + bool includeSpoofedExtension = false, }) { + final TestPackage myApp; + if (includeSpoofedExtension) { + myApp = myAppPackageWithSpoofedExtension; + } else if (includeBadExtension) { + myApp = myAppPackageWithBadExtension; + } else { + myApp = myAppPackage; + } _setupPackage( createTestPackageFrom( - includeBadExtension ? myAppPackageWithBadExtension : myAppPackage, + myApp, includeDependenciesWithExtensions: includeDependenciesWithExtensions, ), isRuntimeRoot: true, @@ -228,7 +242,10 @@ resolution: workspace /// devtools/ /// build/ /// config.yaml - void _setupExtensions({required bool includeBadExtension}) { + void _setupExtensions({ + required bool includeBadExtension, + bool includeSpoofedExtension = false, + }) { _setupExtension(staticExtension1Package); _setupExtension(staticExtension2Package); @@ -237,6 +254,7 @@ resolution: workspace _setupExtension(newerStaticExtension1Package); if (includeBadExtension) _setupExtension(badExtensionPackage); + if (includeSpoofedExtension) _setupExtension(spoofedExtensionPackage); } void _setupPackage(TestPackage package, {bool isRuntimeRoot = false}) { @@ -304,6 +322,10 @@ final myAppPackageWithBadExtension = TestPackage( name: myAppPackage.name, dependencies: [...myAppPackage.dependencies, badExtensionPackage], ); +final myAppPackageWithSpoofedExtension = TestPackage( + name: myAppPackage.name, + dependencies: [...myAppPackage.dependencies, spoofedExtensionPackage], +); final otherRoot1Package = TestPackage( name: 'other_root_1', dependencies: [staticExtension1Package, staticExtension2Package], @@ -374,10 +396,21 @@ final badExtensionPackage = TestPackageWithExtension( isPubliclyHosted: false, packageVersion: null, ); +final spoofedExtensionPackage = TestPackageWithExtension( + name: 'provider', + packageName: 'bad_pkg', + issueTracker: 'https://www.google.com/', + version: '999.0.0', + materialIconCodePoint: 0xe50a, + requiresConnection: true, + isPubliclyHosted: false, + packageVersion: null, +); class TestPackageWithExtension { TestPackageWithExtension({ required this.name, + String? packageName, required this.issueTracker, required this.version, required this.materialIconCodePoint, @@ -385,11 +418,13 @@ class TestPackageWithExtension { required this.isPubliclyHosted, required this.packageVersion, String? relativePathFromExtensions, - }) : assert(isPubliclyHosted == (packageVersion != null)), + }) : packageName = packageName ?? name.toLowerCase(), + assert(isPubliclyHosted == (packageVersion != null)), relativePathFromExtensions = - relativePathFromExtensions ?? name.toLowerCase(); + relativePathFromExtensions ?? (packageName ?? name.toLowerCase()); final String name; + final String packageName; final String issueTracker; final String version; final Object? materialIconCodePoint; @@ -413,7 +448,7 @@ ${!requiresConnection ? 'requiresConnection: false' : ''} String get pubspecContent => ''' -name: ${name.toLowerCase()} +name: $packageName environment: sdk: ">=3.4.0-282.1.beta <4.0.0" '''; @@ -442,7 +477,7 @@ ${_dependenciesAsString()} String _dependenciesAsString() { final sb = StringBuffer(); for (final dep in dependencies) { - sb.write(' ${dep.name.toLowerCase()}:'); + sb.write(' ${dep.packageName}:'); if (dep.isPubliclyHosted) { sb.writeln(' ${dep.packageVersion!}'); } else { diff --git a/packages/devtools_shared/test/server/devtools_extensions_api_test.dart b/packages/devtools_shared/test/server/devtools_extensions_api_test.dart index af0530ed512..4d548675b9e 100644 --- a/packages/devtools_shared/test/server/devtools_extensions_api_test.dart +++ b/packages/devtools_shared/test/server/devtools_extensions_api_test.dart @@ -44,10 +44,12 @@ void main() { Future initializeTestDirectory({ bool includeDependenciesWithExtensions = true, bool includeBadExtension = false, + bool includeSpoofedExtension = false, }) async { await extensionTestManager.setupTestDirectoryStructure( includeDependenciesWithExtensions: includeDependenciesWithExtensions, includeBadExtension: includeBadExtension, + includeSpoofedExtension: includeSpoofedExtension, ); await testDtdConnection!.setIDEWorkspaceRoots(dtd!.info!.secret!, [ extensionTestManager.packagesRootUri, @@ -112,6 +114,37 @@ void main() { ); }); + test( + 'ignores extension whose config.yaml name does not match package name', + () async { + await initializeTestDirectory(includeSpoofedExtension: true); + final response = await serveExtensions(extensionsManager); + expect(response.statusCode, HttpStatus.ok); + _verifyAllExtensions(extensionsManager); + + // Verify that the spoofed extension with version 999.0.0 was not added + // and did not replace the legitimate provider extension. + expect( + extensionsManager.devtoolsExtensions.any( + (e) => e.version == '999.0.0', + ), + isFalse, + ); + + final parsedResponse = + json.decode(await response.readAsString()) as Map; + final warning = + parsedResponse[ExtensionsApi.extensionsResultWarningPropertyName]; + expect( + warning, + contains( + 'Ignoring extension from package "bad_pkg": its config.yaml ' + 'declares name "provider", which does not match the package name.', + ), + ); + }, + ); + test('succeeds for valid extensions when an exception is thrown', () async { await initializeTestDirectory(); extensionsManager = _TestExtensionsManager();