Use a more explicit path to the script file when spawning via cmd.exe

In order to avoid cmd.exe searching the PATH, we make sure the path to the script is as qualified as possible, and we add `.\` in front if it doesn't have any path separators (the presence of a path separator avoids PATH searching).

This is just an optimization/nicety now that we're spawning cmd.exe directly instead of letting CreateProcessW spawn it for us.

Author: squeek502

lib/std/child_process.zig

@@ -1095,7 +1095,7 @@ fn windowsCreateProcessPathExt(
                 }
             };
             const cmd_line_w = if (is_bat_or_cmd)
-                try cmd_line_cache.scriptCommandLine()
+                try cmd_line_cache.scriptCommandLine(full_app_name)
             else
                 try cmd_line_cache.commandLine();
             const app_name_w = if (is_bat_or_cmd)
@@ -1150,7 +1150,7 @@ fn windowsCreateProcessPathExt(
             else => false,
         };
         const cmd_line_w = if (is_bat_or_cmd)
-            try cmd_line_cache.scriptCommandLine()
+            try cmd_line_cache.scriptCommandLine(full_app_name)
         else
             try cmd_line_cache.commandLine();
         const app_name_w = if (is_bat_or_cmd)
@@ -1318,10 +1318,17 @@ pub const WindowsCommandLineCache = struct {
         return self.cmd_line.?;
     }
 
-    pub fn scriptCommandLine(self: *WindowsCommandLineCache) ![:0]u16 {
-        if (self.script_cmd_line == null) {
-            self.script_cmd_line = try argvToScriptCommandLineWindows(self.allocator, self.argv);
-        }
+    /// Not cached, since the path to the batch script will change during PATH searching.
+    /// `script_path` should be as qualified as possible, e.g. if the PATH is being searched,
+    /// then script_path should include both the search path and the script filename
+    /// (this allows avoiding cmd.exe having to search the PATH again).
+    pub fn scriptCommandLine(self: *WindowsCommandLineCache, script_path: []const u16) ![:0]u16 {
+        if (self.script_cmd_line) |v| self.allocator.free(v);
+        self.script_cmd_line = try argvToScriptCommandLineWindows(
+            self.allocator,
+            script_path,
+            self.argv[1..],
+        );
         return self.script_cmd_line.?;
     }
 
@@ -1375,12 +1382,15 @@ pub const ArgvToScriptCommandLineError = error{
 /// Serializes `argv` to a Windows command-line string that uses `cmd.exe /c` and `cmd.exe`-specific
 /// escaping rules. The caller owns the returned slice.
 ///
-/// Escapes `argv` using the suggested mitigation against
-/// arbitrary command execution from:
+/// Escapes `argv` using the suggested mitigation against arbitrary command execution from:
 /// https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/
 pub fn argvToScriptCommandLineWindows(
     allocator: mem.Allocator,
-    argv: []const []const u8,
+    /// Path to the `.bat`/`.cmd` script. If this path is relative, it is assumed to be relative to the CWD.
+    /// The script must have been verified to exist at this path before calling this function.
+    script_path: []const u16,
+    /// Arguments, not including the script name itself. Expected to be encoded as WTF-8.
+    script_args: []const []const u8,
 ) ArgvToScriptCommandLineError![:0]u16 {
     var buf = try std.ArrayList(u8).initCapacity(allocator, 64);
     defer buf.deinit();
@@ -1394,7 +1404,25 @@ pub fn argvToScriptCommandLineWindows(
     // https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/
     buf.appendSliceAssumeCapacity("cmd.exe /d /e:ON /v:OFF /c \"");
 
-    for (argv, 0..) |arg, i| {
+    // Always quote the path to the script arg
+    buf.appendAssumeCapacity('"');
+    // We always want the path to the batch script to include a path separator in order to
+    // avoid cmd.exe searching the PATH for the script. This is not part of the arbitrary
+    // command execution mitigation, we just know exactly what script we want to execute
+    // at this point, and potentially making cmd.exe re-find it is unnecessary.
+    //
+    // If the script path does not have a path separator, then we know its relative to CWD and
+    // we can just put `.\` in the front.
+    if (mem.indexOfAny(u16, script_path, &[_]u16{mem.nativeToLittle(u16, '\\'), mem.nativeToLittle(u16, '/')}) == null) {
+        try buf.appendSlice(".\\");
+    }
+    // Note that we don't do any escaping/mitigations for this argument, since the relevant
+    // characters (", %, etc) are illegal in file paths and this function should only be called
+    // with script paths that have been verified to exist.
+    try std.unicode.wtf16LeToWtf8ArrayList(&buf, script_path);
+    buf.appendAssumeCapacity('"');
+
+    for (script_args) |arg| {
         // Literal carriage returns get stripped when run through cmd.exe
         // and NUL/newlines act as 'end of command.' Because of this, it's basically
         // always a mistake to include these characters in argv, so it's
@@ -1405,7 +1433,7 @@ pub fn argvToScriptCommandLineWindows(
         }
 
         // Separate args with a space.
-        if (i != 0) try buf.append(' ');
+        try buf.append(' ');
 
         // Need to quote if the argument is empty (otherwise the arg would just be lost)
         // or if the last character is a `\`, since then something like "%~2" in a .bat