Don't match libexec/git-core in usr; refactor

- Allowing `usr` as a `<platform>` prefix is more likely to produce
  desired than undesired behavior. But due to the ambiguity of the
  name `usr` on non-Unix systems, maybe this would lead to problems
  that are relevant to security. The concern is described and the
  `usr` prefix, which is never needed to find the shell in a Git
  for Windows installation, is no longer matched.

  Note that this only affects it as a path component that
  `libexec/git-core` is found initially to be in. We do still use

      <prefix>/libexec/git-core/../../../usr/bin/sh.exe

  if we don't find something we can plausibly use at:

      `<prefix>/libexec/git-core/../../../bin/sh.exe

  The helper docstring explains why a security model under which
  this is reasonable does necessarily entail that it is reasonable
  to allow a `<prefix>` of `usr`, *even though* in the path with
  `usr` that we form, it is `usr` in the `<prefix>` position.

  With that said, and even though it does not help find `sh` from
  Git for Windows, hopefully future research can establish that it
  is safe to treat `usr/libexec/git-core` the same as platform
  prefixes like `mingw64/libexec/git-core` and it can be enabled.

- Start refactoring by extracting and renaming the recently
  introduced helper constants and functions.

- Extract most of the `cfg!(windows)` branch in `shell()` itself,
  even with its helpers already extracted, to make it a helper
  function as well.

- Document the recently introduced helper constants.

Author: EliahKagan

gix-path/src/env/mod.rs

@@ -28,6 +28,84 @@ pub fn installation_config_prefix() -> Option<&'static Path> {
     installation_config().map(git::config_to_base_path)
 }
 
+/// `usr`-like directory component names that MSYS2 may provide, other than for `/usr` itself.
+///
+/// These are the values of the "Prefix" column of the "Environments" and "Legacy Environments"
+/// tables in the [MSYS2 Environments](https://www.msys2.org/docs/environments/) documentation,
+/// with the leading `/` separator removed, except that this does not list `usr` itself.
+///
+/// On Windows, we prefer to use `sh` as provided by Git for Windows, when present. To find it, we
+/// run `git --exec-path` to get a path that is usually `<platform>/libexec/git-core` in the Git
+/// for Windows installation, where `<platform>` is something like `mingw64`. It is also acceptable
+/// to find `sh` in an environment not provided by Git for Windows, such as an independent MSYS2
+/// environment in which a `git` package has been installed. However, in an unusual installation,
+/// or if the user has set a custom value of `GIT_EXEC_PATH`, the output of `git --exec-path` may
+/// take a form other than `<platform>/libexec/git-core`, such that finding shell at a location
+/// like `../../../bin/sh.exe` relative to it should not be attempted. We lower the risk by
+/// checking that `<platform>` is a plausible value that is not likely to have any other meaning.
+///
+/// This involves two tradeoffs. First, it may be reasonable to find `sh.exe` in an environment
+/// that is not MSYS2 at all, for which in principle the prefix could be different. But listing
+/// more prefixes or matching a broad pattern of platform-like strings might be too broad. So only
+/// prefixes that have been used in MSYS2 are considered.
+///
+/// Second, we don't recognize `usr` itself here, even though is a plausible prefix. In MSYS2, it
+/// is the prefix for MSYS2 non-native programs, i.e. those that use `msys-2.0.dll`. But unlike the
+/// `<platform>` names we recognize, `usr` also has an effectively unbounded range of plausible
+/// meanings on non-Unix systems, which may occasionally relate to subdirectories whose contents
+/// are controlled by different user accounts.
+///
+/// If we start with a `libexec/git-core` directory that we already use and trust, and it is in a
+/// directory with a name like `mingw64`, we infer that this `mingw64` directory has the expected
+/// meaning and that its `usr` sibling, if present, is acceptable to treat as though it is a
+/// first-level directory inside an MSYS2-like tree. So we are willing to traverse down to
+/// `usr/sh.exe` and attempt to use it. But if the `libexec/git-core` we use and trust is inside a
+/// directory named `usr`, that `usr` directory may still not have the meaning we expect of `usr`.
+///
+/// The conditions for a privilege escalation attack or other serious malfunction seem unlikely. If
+/// research indicates the risk is low enough, `usr` may be added. But for now it is omitted.
+const MSYS_USR_VARIANTS: &[&str] = &["mingw64", "mingw32", "clangarm64", "clang64", "clang32", "ucrt64"];
+
+/// Shell path fragments to concatenate to the root of a Git for Windows or MSYS2 installation.
+///
+/// These look like absolute Unix-style paths, but the leading `/` separators are present because
+/// they simplify forming paths like `C:/Program Files/Git` obtained by removing trailing
+/// components from the output of `git --exec-path`.
+const RAW_SH_EXE_PATH_SUFFIXES: &[&str] = &[
+    "/bin/sh.exe", // Usually a shim, which currently we prefer, if available.
+    "/usr/bin/sh.exe",
+];
+
+///
+fn raw_join(path: &Path, raw_suffix: &str) -> OsString {
+    let mut raw_path = OsString::from(path);
+    raw_path.push(raw_suffix);
+    raw_path
+}
+
+///
+fn find_sh_on_windows() -> Option<OsString> {
+    core_dir()
+        .filter(|core| core.is_absolute() && core.ends_with("libexec/git-core"))
+        .and_then(|core| core.ancestors().nth(2))
+        .filter(|prefix| {
+            // Only use `libexec/git-core` from inside something `usr`-like, such as `mingw64`.
+            MSYS_USR_VARIANTS.iter().any(|name| prefix.ends_with(name))
+        })
+        .and_then(|prefix| prefix.parent())
+        .into_iter()
+        .flat_map(|git_root| {
+            // Enumerate locations where `sh.exe` usually is. To avoid breaking scripts that assume the
+            // shell's own path contains no `\`, and so messages are more readable, append literally
+            // with `/` separators. The path from `git --exec-path` already uses `/` separators (and no
+            // trailing `/`) unless explicitly overridden to an unusual value via `GIT_EXEC_PATH`.
+            RAW_SH_EXE_PATH_SUFFIXES
+                .iter()
+                .map(|raw_suffix| raw_join(git_root, raw_suffix))
+        })
+        .find(|raw_path| Path::new(raw_path).is_file())
+}
+
 /// Return the shell that Git would use, the shell to execute commands from.
 ///
 /// On Windows, this is the full path to `sh.exe` bundled with Git for Windows if we can find it.
@@ -39,43 +117,7 @@ pub fn installation_config_prefix() -> Option<&'static Path> {
 pub fn shell() -> &'static OsStr {
     static PATH: Lazy<OsString> = Lazy::new(|| {
         if cfg!(windows) {
-            const MSYS_PREFIX_NAMES: &[&str] = &[
-                "mingw64",
-                "mingw32",
-                "clangarm64",
-                "clang64",
-                "clang32",
-                "ucrt64",
-                "usr",
-            ];
-            const RAW_SUFFIXES: &[&str] = &[
-                "/bin/sh.exe", // Usually a shim, which currently we prefer, if available.
-                "/usr/bin/sh.exe",
-            ];
-            fn raw_join(path: &Path, raw_suffix: &str) -> OsString {
-                let mut raw_path = OsString::from(path);
-                raw_path.push(raw_suffix);
-                raw_path
-            }
-            core_dir()
-                .filter(|core| core.is_absolute() && core.ends_with("libexec/git-core"))
-                .and_then(|core| core.ancestors().nth(2))
-                .filter(|prefix| {
-                    // Only use `libexec/git-core` from inside something `usr`-like, such as `mingw64`.
-                    MSYS_PREFIX_NAMES.iter().any(|name| prefix.ends_with(name))
-                })
-                .and_then(|prefix| prefix.parent())
-                .into_iter()
-                .flat_map(|git_root| {
-                    // Enumerate the locations where `sh.exe` usually is. To avoid breaking shell
-                    // scripts that assume the shell's own path contains no `\`, and to produce
-                    // more readable messages, append literally with `/` separators. The path from
-                    // `git --exec-path` will already have all `/` separators (and no trailing `/`)
-                    // unless it was explicitly overridden to an unusual value via `GIT_EXEC_PATH`.
-                    RAW_SUFFIXES.iter().map(|raw_suffix| raw_join(git_root, raw_suffix))
-                })
-                .find(|raw_path| Path::new(raw_path).is_file())
-                .unwrap_or_else(|| "sh.exe".into())
+            find_sh_on_windows().unwrap_or_else(|| "sh.exe".into())
         } else {
             "/bin/sh".into()
         }