[hooks_runner] Enforce absolute paths for executables in runProcess

pkgs/hooks_runner/lib/src/build_runner/build_runner.dart

@@ -66,6 +66,8 @@ class NativeAssetsBuildRunner {
   final FileSystem _fileSystemUntraced;
 
   final Logger logger;
+
+  /// Absolute path to the Dart executable used to compile and run hooks.
   final Uri dartExecutable;
   final Duration singleHookTimeout;
   final Map<String, String> hookEnvironment;
@@ -85,6 +87,13 @@ class NativeAssetsBuildRunner {
        hookEnvironment =
            hookEnvironment ??
            filteredEnvironment(includeHookEnvironmentVariable) {
+    if (!dartExecutable.isAbsolute) {
+      throw ArgumentError.value(
+        dartExecutable,
+        'dartExecutable',
+        'Must be an absolute path.',
+      );
+    }
     _fileSystem = TracingFileSystem(fileSystem, _task);
   }
 

pkgs/hooks_runner/lib/src/utils/run_process.dart

@@ -12,10 +12,16 @@ import 'package:logging/logging.dart';
 
 /// Runs a [Process].
 ///
+/// [executable] must be an absolute path. Relative paths and `PATH` lookup are
+/// not supported. On Windows, [executable] must also include a file extension
+/// (for example `.exe`); `PATHEXT` lookup is not supported.
+///
+/// Supports [executable] paths and [arguments] that contain spaces. Never runs
+/// through a shell, so Windows `cmd.exe` quote-stripping does not apply.
+///
 /// If [logger] is provided, stream stdout and stderr to it.
 ///
 /// If [captureOutput], captures stdout and stderr.
-// TODO(dacoharkes): Share between package:native_toolchain_c and here.
 Future<RunProcessResult> runProcess({
   required FileSystem filesystem,
   required Uri executable,
@@ -29,6 +35,8 @@ Future<RunProcessResult> runProcess({
   bool throwOnUnexpectedExitCode = false,
   TimelineTask? task,
 }) async {
+  _validateExecutable(executable);
+
   final printWorkingDir =
       workingDirectory != null &&
       workingDirectory != filesystem.currentDirectory.uri;
@@ -59,21 +67,11 @@ Future<RunProcessResult> runProcess({
       workingDirectory: workingDirectory?.toFilePath(),
       environment: environment,
       includeParentEnvironment: includeParentEnvironment,
-      // On Windows, only launch through `cmd.exe` when strictly necessary.
-      //
-      // `CreateProcess` (used when `runInShell` is `false`) can directly launch
-      // `.exe`/`.com` executables and applies the correct command-line quoting,
-      // so we avoid the shell for those: running them through `cmd.exe /c`
-      // mangles the command line when more than one argument is quoted (cmd
-      // strips the outer quotes when the line contains more than two quote
-      // characters), which breaks any invocation whose executable and arguments
-      // contain spaces.
-      //
-      // However, `CreateProcess` cannot resolve a bare command name to a
-      // `.bat`/`.cmd` shim via `PATHEXT`, nor execute such a shim directly (for
-      // example the executables generated by `dart install`). Those must go
-      // through `cmd.exe`, so we opt into the shell for them.
-      runInShell: Platform.isWindows && _needsShellOnWindows(executable),
+      // Never run through a shell. On Windows, running an executable through
+      // `cmd.exe /c` mangles the command line when more than one argument is
+      // quoted (cmd strips the outer quotes when the line contains more than
+      // two quote characters), which breaks any invocation whose executable
+      // and arguments contain spaces.
     );
 
     final stdoutSub = process.stdout.listen((List<int> data) {
@@ -128,22 +126,31 @@ Future<RunProcessResult> runProcess({
   }
 }
 
-/// Whether [executable] must be launched through `cmd.exe` on Windows.
-///
-/// `CreateProcess` can only directly launch `.exe`/`.com` binaries. Anything
-/// else (a `.bat`/`.cmd` shim, or a bare command name that resolves to one via
-/// `PATHEXT`) has to be run through the shell.
-bool _needsShellOnWindows(Uri executable) {
-  final path = executable.toFilePath();
-  final lastSeparator = path.lastIndexOf(RegExp(r'[\\/]'));
-  final fileName = lastSeparator == -1
-      ? path
-      : path.substring(lastSeparator + 1);
-  final dot = fileName.lastIndexOf('.');
-  // No extension (e.g. a bare command name resolved via `PATHEXT`).
-  if (dot <= 0) return true;
-  final extension = fileName.substring(dot).toLowerCase();
-  return extension != '.exe' && extension != '.com';
+void _validateExecutable(Uri executable) {
+  if (!executable.isAbsolute) {
+    throw ArgumentError.value(
+      executable,
+      'executable',
+      'Must be an absolute path. Relative paths and PATH lookup are not '
+          'supported.',
+    );
+  }
+  // Without a shell, Windows does not apply PATHEXT, so callers must pass the
+  // real file name including its extension (e.g. `dart.exe`, not `dart`).
+  if (Platform.isWindows && !_hasFileExtension(executable.toFilePath())) {
+    throw ArgumentError.value(
+      executable,
+      'executable',
+      'Must include a file extension (e.g. .exe). PATHEXT lookup is not '
+          'supported.',
+    );
+  }
+}
+
+bool _hasFileExtension(String filePath) {
+  final basename = filePath.replaceAll('\\', '/').split('/').last;
+  final dot = basename.lastIndexOf('.');
+  return dot > 0 && dot < basename.length - 1;
 }
 
 /// Drop in replacement of [ProcessResult].

pkgs/hooks_runner/test/build_runner/helpers.dart

@@ -344,11 +344,7 @@ Future<void> expectSymbols({
 }) async {
   if (Platform.isLinux) {
     final assetUri = asset.file!;
-    final nmResult = await runProcess(
-      executable: Uri(path: 'nm'),
-      arguments: ['-D', assetUri.toFilePath()],
-      logger: logger,
-    );
+    final nmResult = await Process.run('nm', ['-D', assetUri.toFilePath()]);
 
     expect(nmResult.stdout, stringContainsInOrder(symbols));
   }

pkgs/hooks_runner/test/helpers.dart

@@ -96,7 +96,8 @@ Future<Uri> tempDirForTest({
 
 /// Runs a [Process].
 ///
-/// If [logger] is provided, stream stdout and stderr to it.
+/// See [run_process.runProcess]. [executable] must be absolute (and include a
+/// file extension on Windows); relative paths and PATHEXT are not supported.
 ///
 /// If [captureOutput], captures stdout and stderr.
 Future<run_process.RunProcessResult> runProcess({

pkgs/hooks_runner/test/utils/run_process_test.dart

@@ -6,11 +6,10 @@
 // contain a space (e.g. the default pub cache under a Windows user name with
 // a space, `C:\Users\First Last\AppData\Local\Pub\Cache\...`).
 //
-// On Windows, `runProcess` only runs through `cmd.exe` (`runInShell`) when
-// strictly necessary: bare command names (resolved via `PATHEXT`) and
-// `.bat`/`.cmd` shims. For `.exe`/`.com` binaries it uses `CreateProcess`
-// directly so command lines with multiple quoted tokens are not mangled by
-// `cmd.exe`'s `/c` quote-stripping rule.
+// `runProcess` never runs through a shell. Absolute paths (with a file
+// extension on Windows) are required; relative paths and PATHEXT are not
+// supported. Spaces in the executable path and arguments are handled by
+// passing them as separate CreateProcess / exec arguments.
 
 import 'dart:io';
 
@@ -77,55 +76,6 @@ void main(List<String> args) {
     expect(result.stdout, contains('ARGV:first arg|second arg'));
   });
 
-  test(
-    'runProcess runs bare .bat shim from working directory on Windows',
-    () async {
-      if (!Platform.isWindows) return;
-
-      final binDir = await tempDirForTest();
-      final batUri = binDir.resolve('test shim.bat');
-      await File.fromUri(batUri).writeAsString(
-        '@echo off\r\necho SHIM_OK %*\r\n',
-      );
-
-      final result = await runProcess(
-        executable: Uri.parse('test shim'),
-        arguments: ['--help'],
-        workingDirectory: binDir,
-        logger: logger,
-      );
-
-      expect(result.exitCode, 0);
-      expect(result.stdout, contains('SHIM_OK'));
-      expect(result.stdout, contains('--help'));
-    },
-  );
-
-  test(
-    'runProcess runs bare .bat shim from PATH on Windows',
-    () async {
-      if (!Platform.isWindows) return;
-
-      final binDir = await tempDirForTest();
-      final batUri = binDir.resolve('test shim.bat');
-      await File.fromUri(batUri).writeAsString(
-        '@echo off\r\necho SHIM_OK %*\r\n',
-      );
-
-      final originalPath = Platform.environment['PATH'] ?? '';
-      final result = await runProcess(
-        executable: Uri.parse('test shim'),
-        arguments: ['--help'],
-        environment: {'PATH': '${binDir.toFilePath()};$originalPath'},
-        logger: logger,
-      );
-
-      expect(result.exitCode, 0);
-      expect(result.stdout, contains('SHIM_OK'));
-      expect(result.stdout, contains('--help'));
-    },
-  );
-
   test('runProcess handles arguments containing a space and quotes', () async {
     final workingDir = await tempDirForTest();
     final result = await runProcess(
@@ -139,4 +89,41 @@ void main(List<String> args) {
     expect(result.stdout, contains('ARGC:2'));
     expect(result.stdout, contains('ARGV:fir"st arg|sec\'ond arg'));
   });
+
+  test('runProcess rejects a relative executable path', () async {
+    await expectLater(
+      runProcess(
+        executable: Uri(path: 'dart'),
+        logger: logger,
+      ),
+      throwsA(
+        isA<ArgumentError>().having(
+          (e) => e.message,
+          'message',
+          contains('absolute'),
+        ),
+      ),
+    );
+  });
+
+  test(
+    'runProcess rejects a Windows executable without a file extension',
+    () async {
+      if (!Platform.isWindows) return;
+
+      await expectLater(
+        runProcess(
+          executable: Uri.file(r'C:\path\to\dart'),
+          logger: logger,
+        ),
+        throwsA(
+          isA<ArgumentError>().having(
+            (e) => e.message,
+            'message',
+            contains('PATHEXT'),
+          ),
+        ),
+      );
+    },
+  );
 }