Repository: go
Issue: syscall: special case `cmd.exe /c <command>` in StartProcess
Labels: OS-Windows,NeedsDecision,compiler/runtime

## Background

It is well known that [os/exec](https://pkg.go.dev/os/exec) doesn't correctly escape nor quote `*.bat`, `*.cmd`, `cmd /c *` arguments. This is because in all three cases the escape/quote rules that apply are the ones defined in [cmd.exe docs](https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/cmd), which are slightly different than the ones `os/exec` and [syscall.StartProcess](https://pkg.go.dev/syscall?GOOS=windows#StartProcess) follow, defined in [parsing-c-command-line-arguments](https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments?view=msvc-170). This discrepancy causes bugs like #1849, #17149, #68313, and can also lead to security vulnerabilities.

The only workaround that exists today to reliably execute `*.bat`, `*.cmd`, `cmd /c *` command using `os/exec` is to manually escape/quote the arguments at caller site and pass the resulting string to [syscall.SysProcAttr.CmdLine](https://pkg.go.dev/syscall?GOOS=windows#SysProcAttr). The problem is that having a robust implementation is complicated, so projects tend to have half-backed solutions, if any.

## Proposal

Special case `%COMSPEC% /c <command>` (%COMSPEC% usually points to `cmd.exe`) by applying the cmd.exe escape/quote rules to `<command>`. The exact rules are left for the implementer, as they are well documented.

Some considerations to take into account:

- `cmd.exe` has two types of quotation rules, let's call it default and special. We should follow the special rule (search for `/s` in the docs), as it is 100% predictable in comparison with the default rule, which has many limitations and can easily fallback to the special rule.
The special rule is simple: if `<command>` starts with a `"`, then the the leading and trailing quotes are stripped. This means that we should always surround `<command>` with quotes and pass `/s` before `/c`.
- `cmd.exe` allows passing multiple cmd-specific parameters before `/c` appears. The command is always what goes after `/c`. Therefore, `cmd.exe /d /c <command>` is valid and we should special-case it.
- `cmd.exe` also execute commands passed after the `/k` parameter. That is used to keep the command processor running after the command is executed, so it doesn't really fit well with the one-shot approach of `syscall.StartProcess`. We can ignore it.

## Why not special case also bat/cmd files

This proposal doesn't attempt to solve issues related to directly executing bat/cmd scripts for the following reasons:

- Windows [CreateProcess](https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw) API explicitly disallows passing the bat/cmd scripts in the application name and recommend to use `cmd /c` instead.
- It is not documented how to reliably detect bat/cmd files. Just using the extension seems brittle. In part this is why `CreateProcess` recommend to use `cmd /c`.
- Rust have been trying to reliably support bat/cmd scripts for more than three years, and the Rust library team recently tried to remove that support due to being difficult to implement correctly: https://github.com/rust-lang/rust/issues/123728.
- `os/exec` will now have a good workaround to execute bat files: `exec.Command("cmd.exe", "/c", "foo.bat", "arg 1")`

Note that I'm not putting this proposal in the proposal process because it is not adding new API nor breaking existing behavior. It is more as an umbrella issue to discuss the design and the implementation.

@golang/security @golang/windows