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) {
