From be823b979773abfc17d9c7ce7f1f9a82428bd7af Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Fri, 31 Jul 2026 04:37:55 +0900 Subject: [PATCH] fix(github,gitlab): find gh/glab config on Windows mise reads forge tokens from the `gh` and `glab` CLI config files as a fallback, but on Windows it only ever looked under `~/.config`, so a logged-in user could still go out unauthenticated and hit the 60/hour anonymous GitHub rate limit. `env::XDG_CONFIG_HOME` has no `#[cfg(windows)]` arm (unlike its `XDG_CACHE_HOME` / `XDG_DATA_HOME` neighbours), so on Windows it resolves to `%USERPROFILE%\.config`, and neither `gh_hosts_path` nor `glab_config_path` had a Windows branch -- only a macOS one. The two CLIs do not even agree on the Windows root, so both had to be checked separately: - go-gh's `ConfigDir()` checks `AppData` explicitly, giving `%APPDATA%\GitHub CLI\hosts.yml` - glab goes through `adrg/xdg`, whose Windows `ConfigHome` is `%LOCALAPPDATA%`, giving `%LOCALAPPDATA%\glab-cli\config.yml` `fj` is unaffected: it resolves through `XDG_DATA_HOME`, which already has a Windows arm. Also: - follow go-gh's order for `gh`: `XDG_CONFIG_HOME` is consulted only when actually set, and `~/.config/gh` becomes the explicit final fallback, as in gh itself - extract `tokens::first_existing_path` so the "first candidate that exists, else the conventional location" rule is one implementation with tests, rather than a copy per forge -- the copies are how the Windows location came to be missing from both - document the real Windows locations, and rewrite the keyring note so a Windows user whose `hosts.yml` has no `oauth_token` is pointed at `mise token github` and `use_git_credentials` / `credential_command` instead Co-Authored-By: Claude Opus 5 --- docs/dev-tools/backends/gitlab.md | 8 ++- docs/dev-tools/github-tokens.md | 31 ++++++++++- src/env.rs | 38 ++++++++++++- src/github.rs | 88 +++++++++++++++++++++++-------- src/gitlab.rs | 49 ++++++++++++----- src/tokens.rs | 72 +++++++++++++++++++++++++ 6 files changed, 248 insertions(+), 38 deletions(-) diff --git a/docs/dev-tools/backends/gitlab.md b/docs/dev-tools/backends/gitlab.md index 78be54a72f8..57097985796 100644 --- a/docs/dev-tools/backends/gitlab.md +++ b/docs/dev-tools/backends/gitlab.md @@ -81,8 +81,12 @@ The legacy `$1`/`${1}` hostname argument is deprecated. Use `MISE_CREDENTIAL_HOS mise can read tokens from [glab](https://gitlab.com/gitlab-org/cli) config as a fallback. It checks: 1. `$GLAB_CONFIG_DIR/config.yml` -2. `$XDG_CONFIG_HOME/glab-cli/config.yml` (defaults to `~/.config/glab-cli/config.yml`) -3. `~/Library/Application Support/glab-cli/config.yml` (macOS) +2. `~/.config/glab-cli/config.yml` — glab's legacy location on every platform, which glab still + prefers when the file is there +3. `$XDG_CONFIG_HOME/glab-cli/config.yml` +4. `~/Library/Application Support/glab-cli/config.yml` (macOS) +5. `%LOCALAPPDATA%\glab-cli\config.yml` (Windows — glab resolves `XDG_CONFIG_HOME` to + `%LOCALAPPDATA%` there, unlike `gh`, which uses `%APPDATA%`) Disable this fallback with: diff --git a/docs/dev-tools/github-tokens.md b/docs/dev-tools/github-tokens.md index 916b9f538ab..6e59a435e91 100644 --- a/docs/dev-tools/github-tokens.md +++ b/docs/dev-tools/github-tokens.md @@ -76,8 +76,10 @@ If you use the [GitHub CLI](https://cli.github.com/) (`gh`), mise can read token mise looks for `hosts.yml` in these locations (first match wins): 1. `$GH_CONFIG_DIR/hosts.yml` -2. `$XDG_CONFIG_HOME/gh/hosts.yml` (defaults to `~/.config/gh/hosts.yml`) +2. `$XDG_CONFIG_HOME/gh/hosts.yml` (when that variable is set) 3. `~/Library/Application Support/gh/hosts.yml` (macOS only) +4. `%APPDATA%\GitHub CLI\hosts.yml` (Windows only — this is gh's default location there) +5. `~/.config/gh/hosts.yml` This is especially useful for **GitHub Enterprise** — the gh CLI stores per-host tokens, so mise can authenticate to multiple GHE instances without juggling environment variables: @@ -193,7 +195,32 @@ mise can use your existing git credential helpers to obtain GitHub tokens. This This is especially useful for: - **Devcontainer environments** where tokens are provided via git credential helpers -- **macOS/Windows** where `gh auth login` stores tokens in the system keyring rather than `hosts.yml` +- **macOS/Windows** where `gh auth login` stores the token in the system keyring (macOS Keychain, + Windows Credential Manager) rather than in `hosts.yml`. In that case `hosts.yml` exists but has no + `oauth_token` key, so reading it cannot help. Run `mise token github` to tell the two apart: if it + prints `(none)` while `gh auth status` works, this setting — or a `github.credential_command` + that shells out to gh — is what you want. + + With a single account, `credential_command = "gh auth token"` is enough. If you authenticate to + more than one host (GitHub Enterprise), pass the host mise is asking about, because bare + `gh auth token` returns the token for gh's _own_ active host. mise exports it as + `MISE_CREDENTIAL_HOST`, and the command runs through the platform's inline shell, so the + interpolation differs: + + On macOS and Linux: + + ```toml + [settings.github] + credential_command = 'gh auth token --hostname "$MISE_CREDENTIAL_HOST"' + ``` + + On Windows, `cmd` is the default inline shell and does not expand `$VAR`: + + ```toml + [settings.github] + credential_command = 'gh auth token --hostname %MISE_CREDENTIAL_HOST%' + ``` + - Any environment where git already has credentials configured mise runs `git credential fill` with `GIT_TERMINAL_PROMPT=0` (to prevent interactive prompts) and caches the result per host for the session. diff --git a/src/env.rs b/src/env.rs index 8b97fc16e48..3952769f1f4 100644 --- a/src/env.rs +++ b/src/env.rs @@ -82,6 +82,20 @@ pub static XDG_DATA_HOME: Lazy = Lazy::new(|| { pub static XDG_STATE_HOME: Lazy = Lazy::new(|| var_path("XDG_STATE_HOME").unwrap_or_else(|| HOME.join(".local").join("state"))); +/// `%LOCALAPPDATA%`. What the `adrg/xdg` Go package resolves `XDG_CONFIG_HOME` to on Windows, +/// so it is where CLIs built on it — `glab` among them — keep their config. +/// +/// Roaming `%APPDATA%` deliberately has no counterpart here. Its only consumer is the `gh` +/// lookup, which has to reproduce go-gh's literal `os.Getenv("AppData")` test — including +/// falling through to `~/.config/gh` when the variable is unset — so a synthesized default +/// would make mise look somewhere gh never would. +/// +/// An empty `%LOCALAPPDATA%` falls back rather than resolving to an empty path — see +/// [`var_path`] — which is also what `adrg/xdg` does (`dir != "" && filepath.IsAbs(dir)`). +#[cfg(windows)] +pub static LOCAL_APPDATA: Lazy = + Lazy::new(|| var_path("LOCALAPPDATA").unwrap_or_else(|| HOME.join("AppData").join("Local"))); + /// control display of "friendly" errors - defaults to release mode behavior unless overridden pub static MISE_FRIENDLY_ERROR: Lazy = Lazy::new(|| { if var_is_true("MISE_FRIENDLY_ERROR") { @@ -662,8 +676,19 @@ pub fn in_home_dir() -> bool { current_dir().is_ok_and(|d| d == *HOME) } +/// The value of `key` as a path, or `None` when it is unset **or empty**. +/// +/// An empty value is not a directory. Without this it would yield an empty `PathBuf`, and every +/// caller joins onto the result — producing a *relative* path that gets resolved against the +/// current working directory. `XDG_CONFIG_HOME=` would make `MISE_CONFIG_DIR` the relative +/// `mise`, and a forge CLI lookup read `gh/hosts.yml` out of whatever directory mise happened to +/// be run from. Treating empty as unset is also what the tools mise mirrors here do: go-gh +/// (`os.Getenv(x) != ""`) and `adrg/xdg` (`dir != "" && filepath.IsAbs(dir)`) both fall through. pub fn var_path(key: &str) -> Option { - var_os(key).map(PathBuf::from).map(replace_path) + var_os(key) + .map(PathBuf::from) + .map(replace_path) + .filter(|p| !p.as_os_str().is_empty()) } /// this returns the environment as if __MISE_DIFF was reversed. @@ -997,6 +1022,17 @@ mod tests { remove_var("MISE_TEST_PATH"); } + /// An empty value is not a directory. Callers all join onto the result, so returning + /// `Some("")` would hand them a relative path resolved against the cwd — e.g. an empty + /// `XDG_CONFIG_HOME` turning `MISE_CONFIG_DIR` into the relative `mise`. + #[tokio::test] + async fn test_var_path_treats_empty_as_unset() { + let _config = Config::get().await.unwrap(); + set_var("MISE_TEST_EMPTY_PATH", ""); + assert_eq!(var_path("MISE_TEST_EMPTY_PATH"), None); + remove_var("MISE_TEST_EMPTY_PATH"); + } + /// vars_safe() must skip pairs whose key or value is not valid UTF-8 rather /// than panicking the way std::env::vars() does (#5370). #[cfg(unix)] diff --git a/src/github.rs b/src/github.rs index 858767fa8a0..f51e2bea90f 100644 --- a/src/github.rs +++ b/src/github.rs @@ -803,30 +803,76 @@ fn read_mise_github_tokens() -> Option> { /// Maps hostname (e.g. "github.com") to oauth_token. static GH_HOSTS: Lazy> = Lazy::new(|| read_gh_hosts().unwrap_or_default()); -/// Resolve the path to gh CLI's hosts.yml, matching gh's own config resolution: -/// 1. $GH_CONFIG_DIR/hosts.yml -/// 2. $XDG_CONFIG_HOME/gh/hosts.yml (env::XDG_CONFIG_HOME handles the fallback to ~/.config) -/// 3. ~/Library/Application Support/gh/hosts.yml (macOS native path from Go's os.UserConfigDir) +/// Resolve the path to gh CLI's hosts.yml, following go-gh's own `ConfigDir()`: +/// 1. `$GH_CONFIG_DIR/hosts.yml` +/// 2. `$XDG_CONFIG_HOME/gh/hosts.yml` — only when that variable is actually set, which is gh's +/// condition; `env::XDG_CONFIG_HOME` defaults to `~/.config` and so cannot express it +/// 3. `%APPDATA%\GitHub CLI\hosts.yml` on Windows — gh checks `AppData` explicitly rather than +/// going through XDG, so this is the default location there and mise never looked at it. +/// Like gh, this branch is taken only when the variable is actually set. +/// 4. `~/.config/gh/hosts.yml`, gh's own last branch, used as the fallback +/// +/// The macOS candidate is kept for compatibility but does not correspond to a gh branch: gh +/// uses `~/.config/gh` on macOS too. fn gh_hosts_path() -> Option { - // Explicit GH_CONFIG_DIR takes priority - if let Ok(dir) = std::env::var("GH_CONFIG_DIR") { + // Explicit GH_CONFIG_DIR takes priority. Empty means unset, as in go-gh's + // `os.Getenv(ghConfigDir) != ""`; `var_os` rather than `var` so a non-UTF-8 directory is + // honoured instead of silently skipped. + if let Some(dir) = std::env::var_os("GH_CONFIG_DIR").filter(|dir| !dir.is_empty()) { return Some(PathBuf::from(dir).join("hosts.yml")); } - // Try XDG path (env::XDG_CONFIG_HOME falls back to ~/.config) - let xdg_path = env::XDG_CONFIG_HOME.join("gh/hosts.yml"); - if xdg_path.exists() { - return Some(xdg_path); - } - // Try macOS native config dir - #[cfg(target_os = "macos")] - { - let macos_path = dirs::HOME.join("Library/Application Support/gh/hosts.yml"); - if macos_path.exists() { - return Some(macos_path); - } - } - // Fall back to XDG path even if it doesn't exist (will produce a trace log) - Some(xdg_path) + + // When XDG_CONFIG_HOME is set it is both gh's next branch and the right thing to name in a + // trace if nothing is found; otherwise the branch gh would fall to on this platform. + // `var_path` treats an empty value as unset, matching go-gh's + // `os.Getenv(xdgConfigHome) != ""`. + let xdg_path = env::var_path("XDG_CONFIG_HOME").map(|dir| dir.join("gh/hosts.yml")); + let fallback = xdg_path.clone().unwrap_or_else(gh_default_hosts_path); + + let candidates = xdg_path + .into_iter() + .chain(gh_native_hosts_paths()) + .collect(); + Some(tokens::first_existing_file(candidates, fallback)) +} + +/// `%APPDATA%\GitHub CLI\hosts.yml`, or `None` when `APPDATA` is unset or empty. +/// +/// go-gh guards that branch with `os.Getenv(appData) != ""` and otherwise falls through to +/// `~/.config/gh`, so the variable being absent is meaningful — synthesizing `~/AppData/Roaming` +/// here would send mise to a directory gh would never have used. +#[cfg(windows)] +fn gh_appdata_hosts_path() -> Option { + std::env::var_os("APPDATA") + .filter(|v| !v.is_empty()) + .map(|v| PathBuf::from(v).join("GitHub CLI/hosts.yml")) +} + +/// Where gh lands when neither `GH_CONFIG_DIR` nor `XDG_CONFIG_HOME` is set. +#[cfg(windows)] +fn gh_default_hosts_path() -> PathBuf { + gh_appdata_hosts_path().unwrap_or_else(|| dirs::HOME.join(".config/gh/hosts.yml")) +} + +#[cfg(not(windows))] +fn gh_default_hosts_path() -> PathBuf { + dirs::HOME.join(".config/gh/hosts.yml") +} + +/// Platform-native locations gh may have written to, probed after the XDG one. +#[cfg(target_os = "macos")] +fn gh_native_hosts_paths() -> Vec { + vec![dirs::HOME.join("Library/Application Support/gh/hosts.yml")] +} + +#[cfg(windows)] +fn gh_native_hosts_paths() -> Vec { + gh_appdata_hosts_path().into_iter().collect() +} + +#[cfg(all(not(target_os = "macos"), not(windows)))] +fn gh_native_hosts_paths() -> Vec { + Vec::new() } fn read_gh_hosts() -> Option> { diff --git a/src/gitlab.rs b/src/gitlab.rs index 3e51807c21b..b3388754747 100644 --- a/src/gitlab.rs +++ b/src/gitlab.rs @@ -362,25 +362,50 @@ static MISE_GITLAB_TOKENS: Lazy> = Lazy::new(|| { static GLAB_HOSTS: Lazy> = Lazy::new(|| read_glab_hosts().unwrap_or_default()); +/// Resolve the path to glab's config.yml, following glab's own `ConfigDir()`: +/// 1. `$GLAB_CONFIG_DIR/config.yml` +/// 2. `$HOME/.config/glab-cli/config.yml` — glab's *legacy* location, which it still prefers when +/// the file is there. glab's `legacyConfigDir()` is `os.UserHomeDir()` + `.config/glab-cli` on +/// every platform, so it is deliberately derived from `HOME` and **not** from +/// `XDG_CONFIG_HOME`: those differ as soon as the user sets that variable. +/// 3. `xdg.ConfigHome/glab-cli/config.yml`, which on Windows is under `%LOCALAPPDATA%`: glab uses +/// the `adrg/xdg` package, and that maps `XDG_CONFIG_HOME` to `%LOCALAPPDATA%` there, not to +/// `%APPDATA%` the way `gh` does fn glab_config_path() -> Option { - if let Ok(dir) = std::env::var("GLAB_CONFIG_DIR") { + // Empty means unset, as in glab's `if glabDir != ""`; `var_os` rather than `var` so a + // non-UTF-8 directory is honoured instead of silently skipped. + if let Some(dir) = std::env::var_os("GLAB_CONFIG_DIR").filter(|dir| !dir.is_empty()) { return Some(PathBuf::from(dir).join("config.yml")); } let xdg_path = env::XDG_CONFIG_HOME.join("glab-cli/config.yml"); - if xdg_path.exists() { - return Some(xdg_path); - } + let candidates: Vec = [ + dirs::HOME.join(".config/glab-cli/config.yml"), + xdg_path.clone(), + ] + .into_iter() + .chain(glab_native_config_paths()) + .collect(); + // Nothing found: name the location glab itself would settle on, which is the last candidate + // (its platform-native XDG config dir). + let fallback = candidates.last().cloned().unwrap_or(xdg_path); + Some(tokens::first_existing_file(candidates, fallback)) +} - #[cfg(target_os = "macos")] - { - let macos_path = dirs::HOME.join("Library/Application Support/glab-cli/config.yml"); - if macos_path.exists() { - return Some(macos_path); - } - } +/// Platform-native locations glab may have written to, probed after the legacy/XDG one. +#[cfg(target_os = "macos")] +fn glab_native_config_paths() -> Vec { + vec![dirs::HOME.join("Library/Application Support/glab-cli/config.yml")] +} + +#[cfg(windows)] +fn glab_native_config_paths() -> Vec { + vec![env::LOCAL_APPDATA.join("glab-cli/config.yml")] +} - Some(xdg_path) +#[cfg(all(not(target_os = "macos"), not(windows)))] +fn glab_native_config_paths() -> Vec { + Vec::new() } fn read_glab_hosts() -> Option> { diff --git a/src/tokens.rs b/src/tokens.rs index c7bd7d30be6..9a7fd69b6e8 100644 --- a/src/tokens.rs +++ b/src/tokens.rs @@ -3,6 +3,7 @@ use crate::env; use serde::Deserialize; use serde_yaml::Value; use std::collections::HashMap; +use std::path::PathBuf; use std::sync::LazyLock as Lazy; use crate::file::path_env_without_shims; @@ -316,10 +317,81 @@ fn token_from_entry(entry: &serde_yaml::Mapping) -> Option { }) } +/// First candidate that is a regular file, else `fallback`. +/// +/// Forge CLIs (`gh`, `glab`) each keep their config under a platform-dependent directory, and +/// mise probes the plausible ones in priority order. Factored out so that order is one rule with +/// one set of tests instead of a copy per forge — the copies are exactly how the Windows location +/// came to be missing from both. +/// +/// `is_file` rather than `exists`: every caller hands the result to `read_to_string`, so a +/// directory that happens to be named `hosts.yml` must not shadow a real config further down the +/// list. Symlinks are followed, so a symlinked config still matches. +pub fn first_existing_file(candidates: Vec, fallback: PathBuf) -> PathBuf { + candidates + .into_iter() + .find(|p| p.is_file()) + .unwrap_or(fallback) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn test_first_existing_file_prefers_the_earliest_that_exists() { + let dir = tempfile::tempdir().unwrap(); + let second = dir.path().join("second"); + let third = dir.path().join("third"); + std::fs::write(&second, "").unwrap(); + std::fs::write(&third, "").unwrap(); + assert_eq!( + first_existing_file( + vec![dir.path().join("first"), second.clone(), third], + dir.path().join("fallback") + ), + second, + "a missing earlier candidate must be skipped, not short-circuit the search" + ); + } + + #[test] + fn test_first_existing_file_skips_a_directory_of_the_same_name() { + // A directory named like the config file must not shadow a real one further down: + // the result is handed straight to `read_to_string`. + let dir = tempfile::tempdir().unwrap(); + let decoy = dir.path().join("early").join("config.yml"); + std::fs::create_dir_all(&decoy).unwrap(); + let real = dir.path().join("late-config.yml"); + std::fs::write(&real, "").unwrap(); + assert_eq!( + first_existing_file(vec![decoy, real.clone()], dir.path().join("fallback")), + real + ); + } + + #[test] + fn test_first_existing_file_falls_back_when_none_exist() { + let dir = tempfile::tempdir().unwrap(); + let fallback = dir.path().join("fallback"); + assert_eq!( + first_existing_file( + vec![dir.path().join("a"), dir.path().join("b")], + fallback.clone() + ), + fallback, + "the fallback is returned even though it does not exist -- callers use it to name \ + the conventional location in a trace message" + ); + } + + #[test] + fn test_first_existing_file_with_no_candidates() { + let dir = tempfile::tempdir().unwrap(); + let fallback = dir.path().join("fallback"); + assert_eq!(first_existing_file(vec![], fallback.clone()), fallback); + } + #[test] fn test_parse_tokens_toml() { let toml = r#"