Check whether FLUTTER_ROOT and FLUTTER_ROOT/bin are writable. (flutter#34291)

jonahwilliams · web-flow · commit 49449505b612 · 2019-06-19T09:12:43.000-07:00
diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart
@@ -95,8 +95,49 @@ class Cache {
   final Directory _rootOverride;
   final List<CachedArtifact> _artifacts = <CachedArtifact>[];
 
+  // Check whether there is a writable bit in the usr permissions.
+  static bool _hasUserWritePermission(FileStat stat) {
+    // First grab the set of permissions for the usr group.
+    final int permissions = ((stat.mode & 0xFFF) >> 6) & 0x7;
+    // These values represent all of the octal permission bits that have
+    // readable and writable permission, though technically if we're missing
+    // readable we probably didn't make it this far.
+    return permissions == 6
+      || permissions == 7;
+  }
+
+  // Unfortunately the memory file system by default specifies a mode of `0`
+  // and is used by the majority of our tests. Default to false and only set
+  // to true when we know it is safe.
+  static bool checkPermissions = false;
+
   // Initialized by FlutterCommandRunner on startup.
-  static String flutterRoot;
+  static String get flutterRoot => _flutterRoot;
+  static String _flutterRoot;
+  static set flutterRoot(String value) {
+    if (value == null) {
+      _flutterRoot = null;
+      return;
+    }
+    if (checkPermissions) {
+      // Verify that we have writable permission in the flutter root. If not,
+      // we're liable to crash in unintuitive ways. This can happen if the user
+      // is using a homebrew or other unofficial channel, or otherwise installs
+      // Flutter into directory without permissions.
+      final FileStat binStat = fs.statSync(fs.path.join(value, 'bin'));
+      final FileStat rootStat = fs.statSync(value);
+      if (!_hasUserWritePermission(binStat) || !_hasUserWritePermission(rootStat)) {
+        throwToolExit(
+          'Warning: Flutter is missing permissions to write files '
+          'in its installation directory - "$value". '
+          'Please install Flutter from an official channel in a directory '
+          'where you have write permissions and that does not require '
+          'administrative or root access. For more information see '
+          'https://flutter.dev/docs/get-started/install');
+      }
+    }
+    _flutterRoot = value;
+  }
 
   // Whether to cache artifacts for all platforms. Defaults to only caching
   // artifacts for the current platform.
diff --git a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart
@@ -343,6 +343,12 @@ class FlutterCommandRunner extends CommandRunner<void> {
     // We must set Cache.flutterRoot early because other features use it (e.g.
     // enginePath's initializer uses it).
     final String flutterRoot = topLevelResults['flutter-root'] ?? defaultFlutterRoot;
+    bool checkPermissions = true;
+    assert(() {
+      checkPermissions = false;
+      return true;
+    }());
+    Cache.checkPermissions = checkPermissions;
     Cache.flutterRoot = fs.path.normalize(fs.path.absolute(flutterRoot));
 
     // Set up the tooling configuration.
@@ -477,10 +483,6 @@ class FlutterCommandRunner extends CommandRunner<void> {
     return EngineBuildPaths(targetEngine: engineBuildPath, hostEngine: engineHostBuildPath);
   }
 
-  static void initFlutterRoot() {
-    Cache.flutterRoot ??= defaultFlutterRoot;
-  }
-
   /// Get the root directories of the repo - the directories containing Dart packages.
   List<String> getRepoRoots() {
     final String root = fs.path.absolute(Cache.flutterRoot);
diff --git a/packages/flutter_tools/test/cache_test.dart b/packages/flutter_tools/test/cache_test.dart
@@ -6,6 +6,8 @@ import 'dart:async';
 
 import 'package:file/file.dart';
 import 'package:file/memory.dart';
+import 'package:flutter_tools/src/base/common.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:mockito/mockito.dart';
 import 'package:platform/platform.dart';
 
@@ -40,7 +42,7 @@ void main() {
       await Cache.lock();
       Cache.checkLockAcquired();
     }, overrides: <Type, Generator>{
-      FileSystem: () => MockFileSystem(),
+      FileSystem: () => FakeFileSystem(),
     });
 
     testUsingContext('should not throw when FLUTTER_ALREADY_LOCKED is set', () async {
@@ -139,12 +141,50 @@ void main() {
     expect(flattenNameSubdirs(Uri.parse('http://docs.flutter.io/foo/bar')), 'docs.flutter.io/foo/bar');
     expect(flattenNameSubdirs(Uri.parse('https://www.flutter.dev')), 'www.flutter.dev');
   }, overrides: <Type, Generator>{
-    FileSystem: () => MockFileSystem(),
+    FileSystem: () => FakeFileSystem(),
+  });
+
+
+  group('Permissions test', () {
+    MockFileSystem mockFileSystem;
+    MockFileStat mockFileStat;
+
+    setUp(() {
+      mockFileSystem = MockFileSystem();
+      mockFileStat = MockFileStat();
+      when(mockFileSystem.path).thenReturn(fs.path);
+      Cache.checkPermissions = true;
+    });
+
+    tearDown(() {
+      Cache.checkPermissions = false;
+    });
+
+    testUsingContext('Throws error if missing usr write permissions in flutterRoot', () {
+      when(mockFileSystem.statSync(any)).thenReturn(mockFileStat);
+      when(mockFileStat.mode).thenReturn(344);
+
+      expect(() => Cache.flutterRoot = '', throwsA(isInstanceOf<ToolExit>()));
+    }, overrides: <Type, Generator>{
+      FileSystem: () => mockFileSystem,
+    }, initializeFlutterRoot: false);
+
+    testUsingContext('Doesnt error if we have usr write permissions in flutterRoot', () {
+      when(mockFileSystem.statSync(any)).thenReturn(mockFileStat);
+      when(mockFileStat.mode).thenReturn(493); // 0755 in decimal.
+
+      Cache.flutterRoot = '';
+    }, overrides: <Type, Generator>{
+      FileSystem: () => mockFileSystem,
+    }, initializeFlutterRoot: false);
+
   });
 }
 
-class MockFileSystem extends ForwardingFileSystem {
-  MockFileSystem() : super(MemoryFileSystem());
+class MockFileSystem extends Mock implements FileSystem {}
+class MockFileStat extends Mock implements FileStat {}
+class FakeFileSystem extends ForwardingFileSystem {
+  FakeFileSystem() : super(MemoryFileSystem());
 
   @override
   File file(dynamic path) {
diff --git a/packages/flutter_tools/test/commands/analyze_continuously_test.dart b/packages/flutter_tools/test/commands/analyze_continuously_test.dart
@@ -6,6 +6,7 @@ import 'dart:async';
 
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/os.dart';
+import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/dart/analysis.dart';
 import 'package:flutter_tools/src/dart/pub.dart';
 import 'package:flutter_tools/src/dart/sdk.dart';
@@ -19,7 +20,7 @@ void main() {
   Directory tempDir;
 
   setUp(() {
-    FlutterCommandRunner.initFlutterRoot();
+    Cache.flutterRoot = FlutterCommandRunner.defaultFlutterRoot;
     tempDir = fs.systemTempDirectory.createTempSync('flutter_analysis_test.');
   });
 
