Ensure that cache dirs and files have appropriate permissions (#28090)

This is a partial re-application of #24669, which was
reverted due to Fuchsia breakages.

https://github.com/flutter/flutter/issues/24413

packages/flutter_tools/lib/src/base/os.dart

@@ -27,7 +27,15 @@ abstract class OperatingSystemUtils {
   OperatingSystemUtils._private();
 
   /// Make the given file executable. This may be a no-op on some platforms.
-  ProcessResult makeExecutable(File file);
+  void makeExecutable(File file);
+
+  /// Updates the specified file system [entity] to have the file mode
+  /// bits set to the value defined by [mode], which can be specified in octal
+  /// (e.g. `644`) or symbolically (e.g. `u+x`).
+  ///
+  /// On operating systems that do not support file mode bits, this will be a
+  /// no-op.
+  void chmod(FileSystemEntity entity, String mode);
 
   /// Return the path (with symlinks resolved) to the given executable, or null
   /// if `which` was not able to locate the binary.
@@ -111,8 +119,24 @@ class _PosixUtils extends OperatingSystemUtils {
   _PosixUtils() : super._private();
 
   @override
-  ProcessResult makeExecutable(File file) {
-    return processManager.runSync(<String>['chmod', 'a+x', file.path]);
+  void makeExecutable(File file) {
+    chmod(file, 'a+x');
+  }
+
+  @override
+  void chmod(FileSystemEntity entity, String mode) {
+    try {
+      final ProcessResult result = processManager.runSync(<String>['chmod', mode, entity.path]);
+      if (result.exitCode != 0) {
+        printTrace(
+          'Error trying to run chmod on ${entity.absolute.path}'
+          '\nstdout: ${result.stdout}'
+          '\nstderr: ${result.stderr}',
+        );
+      }
+    } on ProcessException catch (error) {
+      printTrace('Error trying to run chmod on ${entity.absolute.path}: $error');
+    }
   }
 
   @override
@@ -185,11 +209,11 @@ class _PosixUtils extends OperatingSystemUtils {
 class _WindowsUtils extends OperatingSystemUtils {
   _WindowsUtils() : super._private();
 
-  // This is a no-op.
   @override
-  ProcessResult makeExecutable(File file) {
-    return ProcessResult(0, 0, null, null);
-  }
+  void makeExecutable(File file) {}
+
+  @override
+  void chmod(FileSystemEntity entity, String mode) {}
 
   @override
   List<File> _which(String execName, { bool all = false }) {

packages/flutter_tools/lib/src/cache.dart

@@ -207,8 +207,10 @@ class Cache {
   /// Return a directory in the cache dir. For `pkg`, this will return `bin/cache/pkg`.
   Directory getCacheDir(String name) {
     final Directory dir = fs.directory(fs.path.join(getRoot().path, name));
-    if (!dir.existsSync())
+    if (!dir.existsSync()) {
       dir.createSync(recursive: true);
+      os.chmod(dir, '755');
+    }
     return dir;
   }
 
@@ -280,8 +282,10 @@ class Cache {
     final Directory thirdPartyDir = getArtifactDirectory('third_party');
 
     final Directory serviceDir = fs.directory(fs.path.join(thirdPartyDir.path, serviceName));
-    if (!serviceDir.existsSync())
+    if (!serviceDir.existsSync()) {
       serviceDir.createSync(recursive: true);
+      os.chmod(serviceDir, '755');
+    }
 
     final File cachedFile = fs.file(fs.path.join(serviceDir.path, url.pathSegments.last));
     if (!cachedFile.existsSync()) {
@@ -633,13 +637,16 @@ abstract class EngineCachedArtifact extends CachedArtifact {
     return true;
   }
 
-
   void _makeFilesExecutable(Directory dir) {
-    for (FileSystemEntity entity in dir.listSync()) {
+    os.chmod(dir, 'a+r,a+x');
+    for (FileSystemEntity entity in dir.listSync(recursive: true)) {
       if (entity is File) {
-        final String name = fs.path.basename(entity.path);
-        if (name == 'flutter_tester')
-          os.makeExecutable(entity);
+        final FileStat stat = entity.statSync();
+        final bool isUserExecutable = ((stat.mode >> 6) & 0x1) == 1;
+        if (entity.basename == 'flutter_tester' || isUserExecutable) {
+          // Make the file readable and executable by all users.
+          os.chmod(entity, 'a+r,a+x');
+        }
       }
     }
   }

packages/flutter_tools/test/general.shard/artifacts_test.dart

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/build_info.dart';
@@ -12,102 +13,106 @@ import '../src/common.dart';
 import '../src/context.dart';
 
 void main() {
-  group('CachedArtifacts', () {
-
+  group('Artifacts', () {
+    MemoryFileSystem memoryFileSystem;
     Directory tempDir;
-    CachedArtifacts artifacts;
 
     setUp(() {
-      tempDir = fs.systemTempDirectory.createTempSync('flutter_tools_artifacts_test_cached.');
-      artifacts = CachedArtifacts();
+      memoryFileSystem = MemoryFileSystem();
+      tempDir = memoryFileSystem.systemTempDirectory.createTempSync('artifacts_test.');
     });
 
     tearDown(() {
       tryToDelete(tempDir);
     });
 
-    testUsingContext('getArtifactPath', () {
-      expect(
+    group('CachedArtifacts', () {
+      CachedArtifacts artifacts;
+
+      setUp(() {
+        artifacts = CachedArtifacts();
+      });
+
+      testUsingContext('getArtifactPath', () {
+        expect(
           artifacts.getArtifactPath(Artifact.flutterFramework, platform: TargetPlatform.ios, mode: BuildMode.release),
           fs.path.join(tempDir.path, 'bin', 'cache', 'artifacts', 'engine', 'ios-release', 'Flutter.framework'),
-      );
-      expect(
+        );
+        expect(
           artifacts.getArtifactPath(Artifact.flutterTester),
           fs.path.join(tempDir.path, 'bin', 'cache', 'artifacts', 'engine', 'linux-x64', 'flutter_tester'),
-      );
-    }, overrides: <Type, Generator>{
-      Cache: () => Cache(rootOverride: tempDir),
-      Platform: () => FakePlatform(operatingSystem: 'linux'),
-    });
+        );
+      }, overrides: <Type, Generator>{
+        Cache: () => Cache(rootOverride: tempDir),
+        FileSystem: () => memoryFileSystem,
+        Platform: () => FakePlatform(operatingSystem: 'linux'),
+      });
 
-    testUsingContext('getEngineType', () {
-      expect(
+      testUsingContext('getEngineType', () {
+        expect(
           artifacts.getEngineType(TargetPlatform.android_arm, BuildMode.debug),
           'android-arm',
-      );
-      expect(
+        );
+        expect(
           artifacts.getEngineType(TargetPlatform.ios, BuildMode.release),
           'ios-release',
-      );
-      expect(
+        );
+        expect(
           artifacts.getEngineType(TargetPlatform.darwin_x64),
           'darwin-x64',
-      );
-    }, overrides: <Type, Generator>{
-      Cache: () => Cache(rootOverride: tempDir),
-      Platform: () => FakePlatform(operatingSystem: 'linux'),
+        );
+      }, overrides: <Type, Generator>{
+        Cache: () => Cache(rootOverride: tempDir),
+        FileSystem: () => memoryFileSystem,
+        Platform: () => FakePlatform(operatingSystem: 'linux'),
+      });
     });
-  });
-
-  group('LocalEngineArtifacts', () {
-
-    Directory tempDir;
-    LocalEngineArtifacts artifacts;
 
-    setUp(() {
-      tempDir = fs.systemTempDirectory.createTempSync('flutter_tools_artifacts_test_local.');
-      artifacts = LocalEngineArtifacts(tempDir.path,
-        fs.path.join(tempDir.path, 'out', 'android_debug_unopt'),
-        fs.path.join(tempDir.path, 'out', 'host_debug_unopt'),
-      );
-    });
+    group('LocalEngineArtifacts', () {
+      LocalEngineArtifacts artifacts;
 
-    tearDown(() {
-      tryToDelete(tempDir);
-    });
+      setUp(() {
+        artifacts = LocalEngineArtifacts(tempDir.path,
+          memoryFileSystem.path.join(tempDir.path, 'out', 'android_debug_unopt'),
+          memoryFileSystem.path.join(tempDir.path, 'out', 'host_debug_unopt'),
+        );
+      });
 
-    testUsingContext('getArtifactPath', () {
-      expect(
+      testUsingContext('getArtifactPath', () {
+        expect(
           artifacts.getArtifactPath(Artifact.flutterFramework, platform: TargetPlatform.ios, mode: BuildMode.release),
           fs.path.join(tempDir.path, 'out', 'android_debug_unopt', 'Flutter.framework'),
-      );
-      expect(
+        );
+        expect(
           artifacts.getArtifactPath(Artifact.flutterTester),
           fs.path.join(tempDir.path, 'out', 'android_debug_unopt', 'flutter_tester'),
-      );
-      expect(
-        artifacts.getArtifactPath(Artifact.engineDartSdkPath),
-        fs.path.join(tempDir.path, 'out', 'host_debug_unopt', 'dart-sdk'),
-      );
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform(operatingSystem: 'linux'),
-    });
+        );
+        expect(
+          artifacts.getArtifactPath(Artifact.engineDartSdkPath),
+          fs.path.join(tempDir.path, 'out', 'host_debug_unopt', 'dart-sdk'),
+        );
+      }, overrides: <Type, Generator>{
+        FileSystem: () => memoryFileSystem,
+        Platform: () => FakePlatform(operatingSystem: 'linux'),
+      });
 
-    testUsingContext('getEngineType', () {
-      expect(
+      testUsingContext('getEngineType', () {
+        expect(
           artifacts.getEngineType(TargetPlatform.android_arm, BuildMode.debug),
           'android_debug_unopt',
-      );
-      expect(
+        );
+        expect(
           artifacts.getEngineType(TargetPlatform.ios, BuildMode.release),
           'android_debug_unopt',
-      );
-      expect(
+        );
+        expect(
           artifacts.getEngineType(TargetPlatform.darwin_x64),
           'android_debug_unopt',
-      );
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform(operatingSystem: 'linux'),
+        );
+      }, overrides: <Type, Generator>{
+        FileSystem: () => memoryFileSystem,
+        Platform: () => FakePlatform(operatingSystem: 'linux'),
+      });
     });
   });
 }

packages/flutter_tools/test/general.shard/cache_test.dart

@@ -6,14 +6,20 @@ import 'dart:async';
 
 import 'package:file/file.dart';
 import 'package:file/memory.dart';
+import 'package:file_testing/file_testing.dart';
+import 'package:meta/meta.dart';
 import 'package:mockito/mockito.dart';
 import 'package:platform/platform.dart';
 
 import 'package:flutter_tools/src/cache.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart' show InternetAddress, SocketException;
+import 'package:flutter_tools/src/base/net.dart';
+import 'package:flutter_tools/src/base/os.dart';
 
 import '../src/common.dart';
 import '../src/context.dart';
+import '../src/testbed.dart';
 
 void main() {
   group('$Cache.checkLockAcquired', () {
@@ -51,8 +57,13 @@ void main() {
   });
 
   group('Cache', () {
-    final MockCache mockCache = MockCache();
-    final MemoryFileSystem fs = MemoryFileSystem();
+    MockCache mockCache;
+    MemoryFileSystem memoryFileSystem;
+
+    setUp(() {
+      mockCache = MockCache();
+      memoryFileSystem = MemoryFileSystem();
+    });
 
     testUsingContext('Gradle wrapper should not be up to date, if some cached artifact is not available', () {
       final GradleWrapper gradleWrapper = GradleWrapper(mockCache);
@@ -63,7 +74,7 @@ void main() {
       expect(gradleWrapper.isUpToDateInner(), false);
     }, overrides: <Type, Generator>{
       Cache: ()=> mockCache,
-      FileSystem: () => fs,
+      FileSystem: () => memoryFileSystem,
     });
 
     testUsingContext('Gradle wrapper should be up to date, only if all cached artifact are available', () {
@@ -78,7 +89,7 @@ void main() {
       expect(gradleWrapper.isUpToDateInner(), true);
     }, overrides: <Type, Generator>{
       Cache: ()=> mockCache,
-      FileSystem: () => fs,
+      FileSystem: () => memoryFileSystem,
     });
 
     test('should not be up to date, if some cached artifact is not', () {
@@ -157,6 +168,74 @@ void main() {
   }, overrides: <Type, Generator>{
     FileSystem: () => MockFileSystem(),
   });
+
+  group('EngineCachedArtifact', () {
+    FakeHttpClient fakeHttpClient;
+    FakePlatform fakePlatform;
+    MemoryFileSystem memoryFileSystem;
+    MockCache mockCache;
+    MockOperatingSystemUtils mockOperatingSystemUtils;
+
+    setUp(() {
+      fakeHttpClient = FakeHttpClient();
+      fakePlatform = FakePlatform()..environment = const <String, String>{};
+      memoryFileSystem = MemoryFileSystem();
+      mockCache = MockCache();
+      mockOperatingSystemUtils = MockOperatingSystemUtils();
+      when(mockOperatingSystemUtils.verifyZip(any)).thenReturn(true);
+    });
+
+    testUsingContext('makes binary dirs readable and executable by all', () async {
+      final Directory artifactDir = fs.systemTempDirectory.createTempSync('artifact.');
+      final Directory downloadDir = fs.systemTempDirectory.createTempSync('download.');
+      when(mockCache.getArtifactDirectory(any)).thenReturn(artifactDir);
+      when(mockCache.getDownloadDir()).thenReturn(downloadDir);
+      final FakeCachedArtifact artifact = FakeCachedArtifact(
+        cache: mockCache,
+        binaryDirs: <List<String>>[
+          <String>['bin_dir', 'unused_url_path'],
+        ],
+      );
+      await artifact.updateInner();
+      final Directory dir = memoryFileSystem.systemTempDirectory
+          .listSync(recursive: true)
+          .whereType<Directory>()
+          .singleWhere((Directory directory) => directory.basename == 'bin_dir', orElse: () => null);
+      expect(dir, isNotNull);
+      expect(dir.path, artifactDir.childDirectory('bin_dir').path);
+      verify(mockOperatingSystemUtils.chmod(argThat(hasPath(dir.path)), 'a+r,a+x'));
+    }, overrides: <Type, Generator>{
+      Cache: ()=> mockCache,
+      FileSystem: () => memoryFileSystem,
+      HttpClientFactory: () => () => fakeHttpClient,
+      OperatingSystemUtils: () => mockOperatingSystemUtils,
+      Platform: () => fakePlatform,
+    });
+  });
+}
+
+class FakeCachedArtifact extends EngineCachedArtifact {
+  FakeCachedArtifact({
+    String stampName = 'STAMP',
+    @required Cache cache,
+    Set<DevelopmentArtifact> requiredArtifacts = const <DevelopmentArtifact>{},
+    this.binaryDirs = const <List<String>>[],
+    this.licenseDirs = const <String>[],
+    this.packageDirs = const <String>[],
+  }) : super(stampName, cache, requiredArtifacts);
+
+  final List<List<String>> binaryDirs;
+  final List<String> licenseDirs;
+  final List<String> packageDirs;
+
+  @override
+  List<List<String>> getBinaryDirs() => binaryDirs;
+
+  @override
+  List<String> getLicenseDirs() => licenseDirs;
+
+  @override
+  List<String> getPackageDirs() => packageDirs;
 }
 
 class MockFileSystem extends ForwardingFileSystem {
@@ -179,3 +258,4 @@ class MockRandomAccessFile extends Mock implements RandomAccessFile {}
 class MockCachedArtifact extends Mock implements CachedArtifact {}
 class MockInternetAddress extends Mock implements InternetAddress {}
 class MockCache extends Mock implements Cache {}
+class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}

packages/flutter_tools/test/general.shard/commands/create_usage_test.dart

@@ -10,9 +10,9 @@ import 'package:flutter_tools/src/doctor.dart';
 import 'package:flutter_tools/src/reporting/usage.dart';
 
 import '../../src/common.dart';
+import '../../src/context.dart';
 import '../../src/testbed.dart';
 
-
 void main() {
   group('usageValues', () {
     Testbed testbed;

packages/flutter_tools/test/general.shard/fuchsia/fuchsia_device_test.dart

@@ -401,16 +401,14 @@ void main() {
 
   group('fuchsia app start and stop: ', () {
     MemoryFileSystem memoryFileSystem;
-    MockOperatingSystemUtils osUtils;
+    FakeOperatingSystemUtils osUtils;
     FakeFuchsiaDeviceTools fuchsiaDeviceTools;
     MockFuchsiaSdk fuchsiaSdk;
     setUp(() {
       memoryFileSystem = MemoryFileSystem();
-      osUtils = MockOperatingSystemUtils();
+      osUtils = FakeOperatingSystemUtils();
       fuchsiaDeviceTools = FakeFuchsiaDeviceTools();
       fuchsiaSdk = MockFuchsiaSdk();
-
-      when(osUtils.findFreePort()).thenAnswer((_) => Future<int>.value(12345));
     });
 
     Future<LaunchResult> setupAndStartApp({
@@ -640,8 +638,6 @@ class MockProcessManager extends Mock implements ProcessManager {}
 
 class MockProcessResult extends Mock implements ProcessResult {}
 
-class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}
-
 class MockFile extends Mock implements File {}
 
 class MockProcess extends Mock implements Process {}

packages/flutter_tools/test/general.shard/fuchsia/fuchsia_workflow_test.dart

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import 'package:flutter_tools/src/base/file_system.dart';
-import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/fuchsia/fuchsia_sdk.dart';
 import 'package:flutter_tools/src/fuchsia/fuchsia_workflow.dart';
 import 'package:mockito/mockito.dart';
@@ -11,8 +10,6 @@ import 'package:mockito/mockito.dart';
 import '../../src/common.dart';
 import '../../src/context.dart';
 
-class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}
-
 class MockFile extends Mock implements File {}
 
 void main() {

packages/flutter_tools/test/general.shard/resident_runner_test.dart

@@ -19,6 +19,7 @@ import 'package:json_rpc_2/json_rpc_2.dart';
 import 'package:mockito/mockito.dart';
 
 import '../src/common.dart';
+import '../src/context.dart';
 import '../src/testbed.dart';
 
 void main() {

packages/flutter_tools/test/src/context.dart

@@ -33,8 +33,8 @@ export 'package:flutter_tools/src/base/context.dart' show Generator;
 /// Return the test logger. This assumes that the current Logger is a BufferLogger.
 BufferLogger get testLogger => context.get<Logger>();
 
-MockDeviceManager get testDeviceManager => context.get<DeviceManager>();
-MockDoctor get testDoctor => context.get<Doctor>();
+FakeDeviceManager get testDeviceManager => context.get<DeviceManager>();
+FakeDoctor get testDoctor => context.get<Doctor>();
 
 typedef ContextInitializer = void Function(AppContext testContext);
 
@@ -71,8 +71,8 @@ void testUsingContext(
         name: 'mocks',
         overrides: <Type, Generator>{
           Config: () => buildConfig(fs),
-          DeviceManager: () => MockDeviceManager(),
-          Doctor: () => MockDoctor(),
+          DeviceManager: () => FakeDeviceManager(),
+          Doctor: () => FakeDoctor(),
           FlutterVersion: () => MockFlutterVersion(),
           HttpClient: () => MockHttpClient(),
           IOSSimulatorUtils: () {
@@ -82,10 +82,10 @@ void testUsingContext(
           },
           OutputPreferences: () => OutputPreferences(showColor: false),
           Logger: () => BufferLogger(),
-          OperatingSystemUtils: () => MockOperatingSystemUtils(),
+          OperatingSystemUtils: () => FakeOperatingSystemUtils(),
           SimControl: () => MockSimControl(),
-          Usage: () => MockUsage(),
-          XcodeProjectInterpreter: () => MockXcodeProjectInterpreter(),
+          Usage: () => FakeUsage(),
+          XcodeProjectInterpreter: () => FakeXcodeProjectInterpreter(),
           FileSystem: () => LocalFileSystemBlockingSetCurrentDirectory(),
           TimeoutConfiguration: () => const TimeoutConfiguration(),
         },
@@ -135,7 +135,7 @@ void _printBufferedErrors(AppContext testContext) {
   }
 }
 
-class MockDeviceManager implements DeviceManager {
+class FakeDeviceManager implements DeviceManager {
   List<Device> devices = <Device>[];
 
   String _specifiedDeviceId;
@@ -198,12 +198,12 @@ class MockDeviceManager implements DeviceManager {
   }
 }
 
-class MockAndroidLicenseValidator extends AndroidLicenseValidator {
+class FakeAndroidLicenseValidator extends AndroidLicenseValidator {
   @override
   Future<LicensesAccepted> get licensesAccepted async => LicensesAccepted.all;
 }
 
-class MockDoctor extends Doctor {
+class FakeDoctor extends Doctor {
   // True for testing.
   @override
   bool get canListAnything => true;
@@ -220,7 +220,7 @@ class MockDoctor extends Doctor {
     final List<DoctorValidator> superValidators = super.validators;
     return superValidators.map<DoctorValidator>((DoctorValidator v) {
       if (v is AndroidLicenseValidator) {
-        return MockAndroidLicenseValidator();
+        return FakeAndroidLicenseValidator();
       }
       return v;
     }).toList();
@@ -233,10 +233,13 @@ class MockSimControl extends Mock implements SimControl {
   }
 }
 
-class MockOperatingSystemUtils implements OperatingSystemUtils {
+class FakeOperatingSystemUtils implements OperatingSystemUtils {
   @override
   ProcessResult makeExecutable(File file) => null;
 
+  @override
+  void chmod(FileSystemEntity entity, String mode) { }
+
   @override
   File which(String execName) => null;
 
@@ -273,7 +276,7 @@ class MockOperatingSystemUtils implements OperatingSystemUtils {
 
 class MockIOSSimulatorUtils extends Mock implements IOSSimulatorUtils {}
 
-class MockUsage implements Usage {
+class FakeUsage implements Usage {
   @override
   bool get isFirstRun => false;
 
@@ -314,7 +317,7 @@ class MockUsage implements Usage {
   void printWelcome() { }
 }
 
-class MockXcodeProjectInterpreter implements XcodeProjectInterpreter {
+class FakeXcodeProjectInterpreter implements XcodeProjectInterpreter {
   @override
   bool get isInstalled => true;
 

packages/flutter_tools/test/src/testbed.dart

@@ -9,6 +9,7 @@ import 'dart:typed_data';
 
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/io.dart';
+import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/base/context.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
@@ -33,6 +34,7 @@ final Map<Type, Generator> _testbedDefaults = <Type, Generator>{
       ? FileSystemStyle.windows
       : FileSystemStyle.posix),
   Logger: () => BufferLogger(), // Allows reading logs and prevents stdout.
+  OperatingSystemUtils: () => FakeOperatingSystemUtils(),
   OutputPreferences: () => OutputPreferences(showColor: false), // configures BufferLogger to avoid color codes.
   Usage: () => NoOpUsage(), // prevent addition of analytics from burdening test mocks
   FlutterVersion: () => FakeFlutterVersion() // prevent requirement to mock git for test runner.