Fix getExecutablePath to only return files that are actually readable and executable. (#61)
diff --git a/CHANGELOG.md b/CHANGELOG.md index ec52840..e434226 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md
@@ -1,3 +1,8 @@ +#### 4.1.1 + +* Fixed `getExecutablePath()` to only return path items that are + executable and readable to the user. + #### 4.1.0 * Fix the signatures of `ProcessManager.run`, `.runSync`, and `.start` to be
diff --git a/lib/src/interface/common.dart b/lib/src/interface/common.dart index 2f6a5c9..e0866ad 100644 --- a/lib/src/interface/common.dart +++ b/lib/src/interface/common.dart
@@ -16,7 +16,7 @@ 'windows': 'windows', }; -/// Sanatizes the executable path on Windows. +/// Sanitizes the executable path on Windows. /// https://github.com/dart-lang/sdk/issues/37751 String sanitizeExecutablePath(String executable, {Platform platform = const LocalPlatform()}) { @@ -85,7 +85,26 @@ candidates = _getCandidatePaths(command, searchPath, extensions, context); } for (String path in candidates) { - if (fs.file(path).existsSync()) { + final File candidate = fs.file(path); + FileStat stat = candidate.statSync(); + // Only return files or links that exist. + if (stat.type == FileSystemEntityType.notFound || + stat.type == FileSystemEntityType.directory) { + continue; + } + + const int isExecutable = 0x40; + const int isReadable = 0x100; + const int isExecutableAndReadable = isExecutable | isReadable; + // Should only return files or links that are readable and executable by the + // user. + + // On Windows it's not actually possible to only return files that are + // readable, since Dart reports files that have had read permission removed + // as being readable, but not checking for it is the same as checking for it + // and finding it readable, so we use the same check here on all platforms, + // so that if Dart ever gets fixed, it'll just work. + if (stat.mode & isExecutableAndReadable == isExecutableAndReadable) { return path; } }
diff --git a/pubspec.yaml b/pubspec.yaml index c854b97..cdd3bb3 100644 --- a/pubspec.yaml +++ b/pubspec.yaml
@@ -1,5 +1,5 @@ name: process -version: 4.1.0 +version: 4.1.1 description: A pluggable, mockable process invocation abstraction for Dart. homepage: https://github.com/google/process.dart @@ -12,4 +12,4 @@ platform: '^3.0.0' dev_dependencies: - test: ^1.16.0-nullsafety.8 + test: ^1.16.8
diff --git a/test/src/interface/common_test.dart b/test/src/interface/common_test.dart index cbfd180..f394a81 100644 --- a/test/src/interface/common_test.dart +++ b/test/src/interface/common_test.dart
@@ -2,6 +2,9 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:io' as io; +import 'package:file/local.dart'; +import 'package:path/path.dart' as path; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:platform/platform.dart'; @@ -352,6 +355,87 @@ }); }); }); + group('Real Filesystem', () { + late Platform platform; + late Directory tmpDir; + late Directory pathDir1; + late Directory pathDir2; + late Directory pathDir3; + late Directory pathDir4; + late Directory pathDir5; + late File command1; + late File command2; + late File command3; + late File command4; + late File command5; + const Platform localPlatform = LocalPlatform(); + late FileSystem fs; + + setUp(() { + fs = LocalFileSystem(); + tmpDir = fs.systemTempDirectory.createTempSync(); + pathDir1 = tmpDir.childDirectory('path1')..createSync(); + pathDir2 = tmpDir.childDirectory('path2')..createSync(); + pathDir3 = tmpDir.childDirectory('path3')..createSync(); + pathDir4 = tmpDir.childDirectory('path4')..createSync(); + pathDir5 = tmpDir.childDirectory('path5')..createSync(); + command1 = pathDir1.childFile('command')..createSync(); + command2 = pathDir2.childFile('command')..createSync(); + command3 = pathDir3.childFile('command')..createSync(); + command4 = pathDir4.childFile('command')..createSync(); + command5 = pathDir5.childFile('command')..createSync(); + platform = FakePlatform( + operatingSystem: localPlatform.operatingSystem, + environment: <String, String>{ + 'PATH': <Directory>[ + pathDir1, + pathDir2, + pathDir3, + pathDir4, + pathDir5, + ].map<String>((Directory dir) => dir.absolute.path).join(':'), + }, + ); + }); + + tearDown(() { + tmpDir.deleteSync(recursive: true); + }); + + // This doesn't use the memory filesystem because Dart can't modify file + // executable permissions, so we have to create them with actual commands. + test('Only returns executables in PATH', () { + if (localPlatform.isWindows) { + // Windows doesn't check for executable-ness, and we can't run 'chmod' + // on Windows anyhow. + return; + } + + // Make the second command in the path executable, but not the first. + // No executable permissions + io.Process.runSync("chmod", <String>["0644", "--", command1.path]); + // Only group executable permissions + io.Process.runSync("chmod", <String>["0645", "--", command2.path]); + // Only other executable permissions + io.Process.runSync("chmod", <String>["0654", "--", command3.path]); + // All executable permissions, but not readable + io.Process.runSync("chmod", <String>["0311", "--", command4.path]); + // All executable permissions + io.Process.runSync("chmod", <String>["0755", "--", command5.path]); + + String? executablePath = getExecutablePath( + 'command', + tmpDir.path, + platform: platform, + fs: fs, + ); + + // Make sure that the path returned is for the last command, since that + // one comes last in the PATH, but is the only one executable by the + // user. + _expectSamePath(executablePath, command5.absolute.path); + }); + }); } void _expectSamePath(String? actual, String? expected) {