fix: Don't require usable temp dir to get installation config

When running `git config -l ...` to find the configuration file
path associated with the `git` installation itself, the current
working directory for the subprocess was set to the current
directory prior to GitoxideLabs#1523, and to `/tmp` or a `/tmp`-like directory
since GitoxideLabs#1523 (which improved performance and security).

This builds on GitoxideLabs#1523, as well as on subsequent changes to run `git`
in a way that its behavior depends less on its CWD, by making an
even more robust choice of CWD for the subprocess, so that the CWD
is less likely to be deeply nested or on network storage; more
likely to exist; and, on Unix-like systems, less likely to contain
a `.git` entry (though a `git` with security updates should refuse
to take any configuration from such a repository unless it is owned
by the user).

Due to a combination of other measures that harden against
malicious or unusual contents (especially setting `GIT_DIR`), the
most significant benefit of this change is to fix the problem that
a nonexistent temp dir would prevent the command from succeeding.

The main way that could happen is if `TMPDIR` on Unix-like systems,
or `TMP` or `TEMP` on Windows, is set to an incorrect value.
Because these variables are sometimes reasonable to customize for
specific purposes, it is plausible for them to be set to incorrect
values by accident.

Except on Windows, this always uses `/` as the CWD for the
subprocess.

On Windows, we use the Windows directory (usually `C:\Windows`)
rather than the root of the system drive (usually `C:\`), because:

- We are currently obtaining this information from environment
  variables, and it is possible for our own parent process to pass
  down an overly sanitized environment.

  Although this can be so sanitized we cannot find the Windows
  directory, this is less likely to occur than being unable to find
  the root of the system drive.

  This due to moderately broad awareness that the `SystemRoot`
  environment variable (which, somewhat confusingly, holds the path
  of the Windows directory, not the root of the system drive)
  should be preserved even when clearing most other variables.

  Some libraries will even automatically preserve `SystemRoot` when
  clearing others or restore it. For example:

  * https://go-review.googlesource.com/c/go/+/174318

  As far as I know, such treatment of `SystemDrive` is less common.

And also these two considerations, which are minor by comparison:

- Under the current behavior of `env::temp_dir()`, which is now a
  fallback if we cannot determine the Windows directory, we already
  fall back to the Windows directory evenutally, if temp dir
  related environment variables are also unset.

  This is because `env::temp_dir()` usually calls `GetTempDir2` in
  the Windows API, which implements that fallback behavior (after
  first trying the user's user profile directory).

  Avoiding adding yet another place to fall back to that would not
  otherwise be attempted slightly decreases behavioral complexity,
  and there is no reason to think a directory like `C:\` would work
  when a directory like `C:\Windows` doesn't.

- The root of the system drive on a Windows system usually permits
  limited user accounts to create new directories there, so a
  directory like `C:\` on Windows actually has most of the
  disadvantages of a location like `/tmp` on a Unix-like system.

  * GHSA-vw2c-22j4-2fh2
  * GHSA-mgvv-9p9g-3jv4

  This is actually a much less significant reason to prefer a
  directory like `C:\Windows` to a directory like `C:\` than it
  might seem. After all, if `C:\.git` exists and and `git` uses it
  when run from `C:\`, then `git` would usually also use it when
  run from `C:\Windows` (and from numerous other locations)!

  However, the reason there is still a small reason to prefer a
  location like `C:\Windows` to a location like `C:\` is that, if a
  system has a vulnerable `git` but a user or system administrator
  has sought to work around it by listing `C:\` in
  `GIT_CEILING_DIRECTORIES`, then that may keep `git` from
  traversing upward into `C:\`, but it would not keep `C:\` from
  being used if that is where we already are.

  An even more significant reason this motivation is a minor one is
  that the other measures we are taking, including setting
  `GIT_DIR`, should be sufficient to avoid at least the security
  dimension of the problem, which arises from actually using the
  configuration from a repo that is discovered.

The reason we do not prefer the user's user profile directory is:

- The user profile directory may be more deeply nested.

- The user profile directory may sometimes be on slow network
  storage when the discovered Windows directory is not.

- In some situations, the user profile directory does not actually
  exist, or does not exist yet.

- Overly sanitized environments are more likely to lack the
  `USERPROFILE` vairable than the `SystemRoot` variable.

- Users may occasionally choose to have their entire user profile
  directory be a Git repository.

- It's no easier to avoid the problem of using `C:\.git` in a user
  profile directory than in `C:\Windows`: they're usually both under
  `C:\`, and are both not the same as `C:\`. (If the user profile
  directory is a repository, then that will avoid that problem, yet
  be its own problem, if not for other measures that prevent both.)

Author: EliahKagan

gix-path/src/env/git/mod.rs

@@ -109,12 +109,24 @@ fn exe_info() -> Option<BString> {
 
 fn git_cmd(executable: PathBuf) -> Command {
     let mut cmd = Command::new(executable);
+
     #[cfg(windows)]
     {
         use std::os::windows::process::CommandExt;
         const CREATE_NO_WINDOW: u32 = 0x08000000;
         cmd.creation_flags(CREATE_NO_WINDOW);
+
+        cmd.current_dir(
+            env::var_os("SystemRoot") // Most reliable env var for path to Windows directory.
+                .or_else(|| env::var_os("windir")) // Less reliable, but some callers are unusual.
+                .map(PathBuf::from)
+                .filter(|p| p.is_absolute())
+                .unwrap_or_else(env::temp_dir),
+        );
     }
+    #[cfg(not(windows))]
+    cmd.current_dir("/");
+
     // Git 2.8.0 and higher support --show-origin. The -l, -z, and --name-only options were
     // supported even before that. In contrast, --show-scope was introduced later, in Git 2.26.0.
     // Low versions of git are still sometimes used, and this is sometimes reasonable because
@@ -125,12 +137,13 @@ fn git_cmd(executable: PathBuf) -> Command {
     // system scope are possible. This commonly happens on macOS with Apple Git, where the config
     // file under /Library is shown as an "unknown" scope but takes precedence over the system
     // scope. Although GIT_CONFIG_NOSYSTEM will suppress this as well, passing --system omits it.
+    //
     cmd.args(["config", "-lz", "--show-origin", "--name-only"])
-        .current_dir(env::temp_dir())
         .env("GIT_DIR", NULL_DEVICE) // Avoid getting local-scope config.
         .env("GIT_WORK_TREE", NULL_DEVICE) // Not needed, but clarifies intent.
         .stdin(Stdio::null())
         .stderr(Stdio::null());
+
     cmd
 }
 

gix-path/src/env/git/tests.rs

@@ -407,6 +407,10 @@ fn set_temp_env_vars<'a>(path: &Path) -> gix_testtools::Env<'a> {
     env
 }
 
+fn unset_windows_directory_vars<'a>() -> gix_testtools::Env<'a> {
+    gix_testtools::Env::new().unset("windir").unset("SystemRoot")
+}
+
 fn check_exe_info() {
     let path = super::exe_info()
         .map(crate::from_bstring)
@@ -436,6 +440,15 @@ fn exe_info_tolerates_broken_temp() {
     check_exe_info();
 }
 
+#[test]
+#[serial]
+fn exe_info_tolerates_oversanitized_env() {
+    // This test runs on all systems, but it is a regression test for Windows.
+    // Also, having both a broken temp dir and an over-sanitized environment is not supported.
+    let _env = unset_windows_directory_vars();
+    check_exe_info();
+}
+
 #[test]
 #[serial]
 fn exe_info_same_result_with_broken_temp() {
@@ -450,6 +463,20 @@ fn exe_info_same_result_with_broken_temp() {
     assert_eq!(with_unmodified_temp, with_nonexistent_temp);
 }
 
+#[test]
+#[serial]
+fn exe_info_same_result_with_oversanitized_env() {
+    // See `exe_info_tolerates_oversanitized_env`.
+    let with_unmodified_env = super::exe_info();
+
+    let with_oversanitized_env = {
+        let _env = unset_windows_directory_vars();
+        super::exe_info()
+    };
+
+    assert_eq!(with_unmodified_env, with_oversanitized_env);
+}
+
 #[test]
 #[serial]
 fn exe_info_never_from_local_scope() {