[slix]

Summary

On native Windows, git_shell_path() assumes that locate_in_PATH("sh")
always succeeds and passes its result directly to convert_slashes(). If
sh is absent from PATH, the lookup returns NULL and Git terminates
with an access violation.

Check the lookup result and report a fatal error instead:

fatal: cannot find 'sh' in PATH

This prevents the NULL dereference. It does not add a shell fallback, so
commands requiring a shell still fail when no shell is available.

Crash analysis

The failure was reproduced in git-remote-https.exe from Git for Windows
2.55.0.windows.3. ProcDump captured the unhandled exception:

Process:               git-remote-https.exe (97196)
Process image:         C:\Program Files\Git\mingw64\libexec\git-core\git-remote-https.exe
Exception monitor:     Unhandled

[21:32:43]Exception: C0000005.ACCESS_VIOLATION
[21:32:43]Unhandled: C0000005.ACCESS_VIOLATION
[21:32:43]Dump 1 initiated: D:\Libraries\Downloads\git-remote-https-crash.dmp
[21:32:44]Dump 1 writing: Estimated dump file size is 47 MB.
[21:32:44]Dump 1 complete: 47 MB written in 0.6 seconds

The dump confirms that the faulting instruction attempted to read through
RAX while RAX was zero:

C0000005.ACCESS_VIOLATION
RAX=0000000000000000
movzx edx, byte ptr [rax]

Correlation of the surrounding disassembly with the source shows it
loading "sh", performing the PATH lookup, and then entering the slash
conversion. This matches the native Windows implementation:

char *p = locate_in_PATH("sh");
convert_slashes(p);

The reproduced missing-sh condition therefore leaves p as NULL,
which is dereferenced by convert_slashes().

Regression test

Add a MINGW-only regression test that clears PATH inside
test-tool run-command, calls git_shell_path(), and verifies that Git
reports the controlled fatal diagnostic instead of crashing.

The helper clears PATH after process startup, making the missing-shell
condition deterministic without depending on Windows startup PATH
reconstruction.

Testing

Passed:

• git diff --check
• Shell syntax check for t/t0061-run-command.sh
• Added-line formatting and commit-message checks

The native Windows test suite was not run locally because a Git for
Windows SDK/compiler was unavailable in the test environment. Windows CI
is expected to build the change and run t0061-run-command.sh.

[dscho]

On native Windows, git_shell_path() assumes that locate_in_PATH("sh")
always succeeds and passes its result directly to convert_slashes(). If
sh is absent from PATH, the lookup returns NULL and Git terminates
with an access violation.

Running Git for Windows in an environment where sh is absent from the PATH is an unsupported scenario. git.exe specifically ensures that /usr/bin/ (where sh.exe resides unless you willfully remove it, and with it, remove yourself from any supported use case) is present in the PATH. The git-remote-https.exe helper is private to Git, and its only supported use is by being called from git.exe. Therefore, sh.exe is always guaranteed to be in the PATH when git-remote-https.exe is running, unless the installation has been corrupted.

Therefore, this PR looks like a fix for something that is not considered a bug, but an incorrect usage of git-remote-https.exe.

[rimrul]

Running Git for Windows in an environment where sh is absent from the PATH is an unsupported scenario.

While that is true, I still think clearly telling the user that they messed up their setup is nicer than crashing with a SIGSEGV.

[dscho]

While this is definitely outside of the supported use cases of Git for Windows, I am sympathetic to the desire. Maybe these two concerns could be addressed?

[dscho]

Hmm. Is it not possible to do the very same thing in a lot less lines, in a subshell? (unset PATH && git var GIT_SHELL_PATH) should do the same job, without changes to the t/helper/ code, no?

[dscho]

This function is called in two places:

• from git var GIT_SHELL_PATH, and
• from prepare_shell_cmd().

This latter code path, prepare_shell_cmd(), worries me. It's right smack down in the libified part of Git, i.e. it's supposed to return error() and never simply exit() (which die() does). It's called from:

• [prepare_cmd()]https://github.com/git-for-windows/git/blob/v2.56.0-rc2.windows.1/run-command.c#L424), which is called from start_command(), and from
• directly from start_command()

Of course, one could argue that this start_command() function is definitely not thoroughly libified, calling child_die() in a couple of circumstances. But I don't think it'd be a good idea to add on top of the challenges to properly make this a library function that callers can rely on cleaning up resources and returning proper errors instead of simply taking down the entire process. That would make the ongoing libification effort even harder than it already is.

[dscho]

A better alternative to die() might be to hard-code the absolute path to sh.exe in git_exec_path() (via a static char * that is initialized to xstrfmt( "%s/sh.exe", git_exec_path()) if needed and still NULL). That way, the process will be started, and it will fail with exit code 127 which is at least catchable by the callers.

[dscho]

#6475 seems to be helped by this, so I am a lot more willing to merge this (after addressing the libification concern).