fix: force file-based keyring on WSL to prevent grant login hanging

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Fixed
+
+- `grant login` no longer hangs on WSL2; grant now detects WSL and forces the file-based keyring. A token stored in the OS keyring becomes invisible — re-run `grant login` if prompted
+
 ### Added
 
 - End-to-end self-update tests that replace a real, running binary (`internal/selfupdate/e2e_test.go`, build tag `selfupdate_e2e`). They compile two fixture binaries from a dependency-free module, execute one, and swap it through grant's own apply path while a process is still running from that image — so the Windows file-locking semantics behind the two-rename swap are actually exercised, not just the bookkeeping. Success and rollback paths are both covered, and the rolled-back binary is asserted to still run. No network access is required

CLAUDE.md

@@ -157,6 +157,27 @@ Custom `SCAAccessService` follows SDK conventions:
 - SDK profile: `~/.idsec/profiles/grant` (default; override via `IDSEC_PROFILES_FOLDER`)
 - Always resolve the profile directory with `profiles.GetProfilesFolder()` (SDK) — never hand-roll it. The SDK reads `os.Getenv("HOME")`, not `os.UserHomeDir()`; on Windows `HOME` is frequently unset, so it resolves to a **relative** `.idsec/profiles` under the process CWD. Any code that prints or computes the profile path must agree with the loader, so reproduce the SDK's behavior rather than "correcting" it
 
+## Keyring
+- The SDK stores the auth token via `pkg/common/keyring`. `GetKeyring(enforceBasic bool)` (`idsec_keyring.go:117-131`) picks: basic (file) keyring if Docker **or** its own `isWSL()` **or** `IDSEC_BASIC_KEYRING != ""` **or** `enforceBasic`; else the OS keyring on windows/darwin, and on linux the OS-provided (D-Bus/libsecret) keyring whenever `DBUS_SESSION_BUS_ADDRESS` is non-empty; else basic
+- **SDK bug (v0.8.1):** `isWSL()` (`:90-95`) matches `"Microsoft"` **case-sensitively** against `/proc/version`. Modern WSL2 reports `-microsoft-standard-WSL2`, so it returns false. Under WSLg `DBUS_SESSION_BUS_ADDRESS` is set, the D-Bus keyring is chosen, and the call can block forever against an unresponsive `gnome-keyring-daemon`
+- **The SDK's fallbacks cannot save you.** Both the OS-keyring wrapper and `SaveToken` (`:154-171`) fall back to the basic keyring only when the underlying call returns an **error**. A hang is not an error, so the fallback is unreachable. The hang is also not write-only — `LoadToken`/`GetPassword` can wedge before anything is ever written
+- **`IDSEC_BASIC_KEYRING` semantics:** the SDK tests `os.Getenv(...) != ""`, so **any** non-empty value forces the file keyring, `0` and `false` included. Empty/unset does not itself force it — the other `GetKeyring` conditions (Docker, SDK-detected WSL, `enforceBasic`) still apply
+- **grant's override:** `internal/keyringenv` detects WSL and sets `IDSEC_BASIC_KEYRING=1`. A non-empty existing value is preserved; an explicitly **empty** value is overwritten on WSL, because empty is precisely the dangerous setting. No-op off linux (the `GOOS` guard short-circuits before any filesystem access)
+- Signals, OR'd, in the order the diagnostic reports them: `/run/WSL` exists → `/proc/sys/fs/binfmt_misc/WSLInterop` exists → `/proc/sys/kernel/osrelease` contains `microsoft`/`wsl` → `/proc/version` contains `microsoft` → non-empty `WSL_DISTRO_NAME`/`WSL_INTEROP`. All string matching is case-insensitive
+- **Why the token sets differ:** `osrelease` is short and structured, so `wsl` is safe there — systemd matches `Microsoft`/`WSL` on it in `src/basic/virt.c`. (npm `is-wsl` is *not* a precedent for the `wsl` token: it matches only `microsoft`, on `os.release()` then `/proc/version`, before falling back to `WSLInterop`/`/run/WSL`, with everything gated behind `!isInsideContainer()`. It is a precedent for the markers.) `/proc/version` is free-form and carries the kernel build user, build host and compiler banner, so a bare `wsl` there would match a plain Linux box built on a host named `wsl-builder` — `microsoft` only
+- **`/run/WSL` is the strongest single signal**, not the string matching: it survives custom kernels, `sudo`, systemd units and cron. `WSL_DISTRO_NAME` is lost under `sudo -i` (microsoft/WSL#5914) and absent in systemd units (#9719), and custom WSL2 kernels may carry neither token (#6911). snapd abandoned string matching for this marker after Launchpad #1991823. WSL's init creates it in `InteropServer::Create()` (`src/linux/init/util.cpp`) from an unguarded call in `ConfigInitializeInstance()` (`src/linux/init/config.cpp`), so it appears even when interop is disabled
+- **`WSLInterop` is a supplement only** — but not for the reason previously recorded here. The old claim was "that binfmt entry exists only when interop is enabled". That is true on **WSL1**, where the per-distro registration is gated on `Config.InteropEnabled` (`src/linux/init/config.cpp:543-551`); on WSL2 the entry is registered at VM level and is kernel-global, so it is instead vulnerable to being wiped or shadowed VM-wide
+- **All five signals are deliberately redundant, and the redundancy is about *visibility*, not reliability.** This is the point that stops someone simplifying this again — an attempt in `db73645` was reverted in `23ac108` for exactly this reason. The five sources fail **independently** because they are reached by different mechanisms:
+  - filesystem markers (`/run/WSL`, `WSLInterop`) are **namespace-local** — a chroot or mount namespace with a clean `/run`, or without `binfmt_misc` mounted, sees neither, no matter what init did
+  - proc paths (`osrelease`, `/proc/version`) are **independently maskable** — they are separate mount-visible paths, so one can be masked, omitted or replaced while the other is readable
+  - env vars (`WSL_DISTRO_NAME`, `WSL_INTEROP`) are the only ones **inherited across** chroot and mount-namespace boundaries, which is precisely where every marker above disappears
+- **Content equivalence is not signal redundancy.** `/proc/version` and `osrelease` provably cannot *disagree*: `fs/proc/version.c` does `seq_printf(m, linux_proc_banner, utsname()->sysname, utsname()->release, utsname()->version)`, and `/proc/sys/kernel/osrelease` **is** `utsname()->release` (`kernel/utsname_sysctl.c`, `uts_kern_table` entry `osrelease` → `init_uts_ns.name.release`, resolved per-namespace by `get_uts()`); a UTS namespace change moves both together. Useful for reasoning about the *values*, but **not grounds for removing either** — identical content is no help when one path is not visible. Same shape of argument for the env vars: init sets `WSL_DISTRO_NAME` and creates `/run/WSL` in the same unguarded function, which proves init made the marker, **not** that a descendant process can still see it
+- **Error asymmetry drives the tuning:** a false negative attempts the OS keyring under WSLg and hangs indefinitely with no error and no timeout (unrecoverable); a false positive is a file keyring on a plain Linux desktop (bounded security downgrade). Bias toward over-detection. A redundant check is the cheap side of that trade — do not trade it away for tidiness
+- Rejected: container gating (`/run/WSL` is container-safe since a container gets its own `/run` tmpfs). Note the *old* justification — "inside a container `DBUS_SESSION_BUS_ADDRESS` is rarely set, so the SDK already picks basic" — is **refuted**: WSL init injects `DBUS_SESSION_BUS_ADDRESS` explicitly for systemd-backed launches (`src/linux/init/init.cpp`), and ordinary `chroot` preserves it, so the SDK cannot be assumed to fall back on its own. Also rejected: `WSLENV` (user-configurable, often absent), shelling out to `systemd-detect-virt`, and third-party libs (`gookit/goutil` has the same case-sensitive `"Microsoft"` bug)
+- Applied in `executeWithKeyringOverride` (`cmd/root.go`), called from `Execute()` before `rootCmd.Execute()` — the single deterministic entry point for the binary, ahead of every keyring access (`cmd/login.go`, `cmd/root.go`, `cmd/logout.go`). It **fails closed**: a `Setenv` error aborts before any command runs
+- The applied notice is stashed in `keyringEnvNotice` and emitted from `PersistentPreRunE` inside the `if verbose` branch, so it is verbose-only and unit-testable with `spyLogger` (gating on `IDSEC_LOG_LEVEL` alone would be invisible to the spy)
+- Backend switch caveat: a token written to the OS keyring is invisible to the file keyring. Worst case the user re-runs `grant login`
+
 ## Authentication
 - Use the `/grant-login` skill when you need to authenticate to the grant CLI (e.g., before manual testing)
 - Skill definition: `.claude/skills/grant-login/SKILL.md`

README.md

@@ -174,6 +174,7 @@ favorites:
 |----------|-------------|---------|
 | `GRANT_CONFIG` | Custom path to app config YAML | `~/.grant/config.yaml` |
 | `IDSEC_LOG_LEVEL` | SDK log level (`DEBUG`, `INFO`, `CRITICAL`) — overrides `--verbose` | Not set |
+| `IDSEC_BASIC_KEYRING` | Store the auth token in the SDK's encrypted file keyring instead of the OS keyring. **Any non-empty value forces file storage — including `0` and `false`.** Empty or unset does not itself force it (the SDK still picks file storage in Docker and in the cases it detects as WSL). grant sets it to `1` automatically when it detects WSL; an existing non-empty value is never overridden | Not set (auto-set to `1` on WSL) |
 
 ## Troubleshooting
 
@@ -184,6 +185,7 @@ favorites:
 | "Failed to elevate" | Check `grant status` for active sessions; verify target/role names |
 | `grant env` errors for Azure/GCP | `env` is AWS-only — Azure and GCP return no credentials, use `grant` directly |
 | Permission denied accessing keyring (Linux) | Install and start `gnome-keyring` or `kwalletmanager` |
+| `grant login` hangs forever with no output (Linux/WSL) | The OS keyring (D-Bus/libsecret) is unresponsive and the call never returns. grant forces the file keyring on WSL automatically — run with `--verbose` to confirm. On other Linux hosts, set `IDSEC_BASIC_KEYRING=1` and retry. Switching backends hides any token already stored in the OS keyring; just re-run `grant login` |
 
 ## Development
 

cmd/keyring_override_test.go

@@ -0,0 +1,156 @@
+package cmd
+
+import (
+	"errors"
+	"io"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+// resetKeyringOverrideState restores the package globals this file mutates.
+func resetKeyringOverrideState(t *testing.T) {
+	t.Helper()
+	origApply := keyringApply
+	origNotice := keyringEnvNotice
+	origVerbose := verbose
+	origLog := log
+	t.Cleanup(func() {
+		keyringApply = origApply
+		keyringEnvNotice = origNotice
+		verbose = origVerbose
+		log = origLog
+	})
+}
+
+// TestExecuteWithKeyringOverrideRunsBeforeCommand proves the keyring override
+// is applied at startup, before any command code executes.
+func TestExecuteWithKeyringOverrideRunsBeforeCommand(t *testing.T) {
+	resetKeyringOverrideState(t)
+
+	var order []string
+	keyringApply = func() (bool, string, error) {
+		order = append(order, "keyring-override")
+		return true, "WSL detected (test); forcing file-based keyring", nil
+	}
+	keyringEnvNotice = ""
+
+	cmd := newRootCommand(func(*cobra.Command, []string) error {
+		order = append(order, "command-run")
+		return nil
+	})
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	cmd.SetArgs([]string{})
+
+	if err := executeWithKeyringOverride(cmd); err != nil {
+		t.Fatalf("executeWithKeyringOverride() error = %v", err)
+	}
+
+	want := []string{"keyring-override", "command-run"}
+	if len(order) != len(want) || order[0] != want[0] || order[1] != want[1] {
+		t.Fatalf("execution order = %v, want %v", order, want)
+	}
+	if keyringEnvNotice == "" {
+		t.Error("keyringEnvNotice was not stashed after the override applied")
+	}
+}
+
+func TestExecuteWithKeyringOverrideFailsClosed(t *testing.T) {
+	resetKeyringOverrideState(t)
+
+	keyringApply = func() (bool, string, error) {
+		return false, "", errors.New("setenv denied")
+	}
+	keyringEnvNotice = ""
+
+	ran := false
+	cmd := newRootCommand(func(*cobra.Command, []string) error {
+		ran = true
+		return nil
+	})
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	cmd.SetArgs([]string{})
+
+	err := executeWithKeyringOverride(cmd)
+	if err == nil {
+		t.Fatal("executeWithKeyringOverride() error = nil, want an error")
+	}
+	if !strings.Contains(err.Error(), "setenv denied") {
+		t.Errorf("error = %q, want it to wrap the underlying cause", err)
+	}
+	if ran {
+		t.Error("command executed despite the keyring override failing; want fail-closed")
+	}
+}
+
+func TestExecuteWithKeyringOverrideNoNoticeWhenNotApplied(t *testing.T) {
+	resetKeyringOverrideState(t)
+
+	keyringApply = func() (bool, string, error) { return false, "not WSL", nil }
+	keyringEnvNotice = ""
+
+	cmd := newRootCommand(func(*cobra.Command, []string) error { return nil })
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	cmd.SetArgs([]string{})
+
+	if err := executeWithKeyringOverride(cmd); err != nil {
+		t.Fatalf("executeWithKeyringOverride() error = %v", err)
+	}
+	if keyringEnvNotice != "" {
+		t.Errorf("keyringEnvNotice = %q, want empty when the override was not applied", keyringEnvNotice)
+	}
+}
+
+// TestKeyringNoticeIsVerboseOnly checks that the stashed notice reaches the
+// logger only when --verbose is passed.
+//
+// Note: the spy logger records Info() calls regardless of the SDK log level, so
+// gating purely on IDSEC_LOG_LEVEL would be invisible to this test. The
+// implementation therefore gates the emission on the parsed --verbose flag
+// itself, which is what this test asserts.
+func TestKeyringNoticeIsVerboseOnly(t *testing.T) {
+	tests := []struct {
+		name     string
+		args     []string
+		notice   string
+		wantLogs bool
+	}{
+		{name: "verbose emits the notice", args: []string{"--verbose"}, notice: "WSL detected; forcing file-based keyring", wantLogs: true},
+		{name: "non-verbose stays silent", args: []string{}, notice: "WSL detected; forcing file-based keyring", wantLogs: false},
+		{name: "verbose with no notice logs nothing", args: []string{"--verbose"}, notice: "", wantLogs: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			resetKeyringOverrideState(t)
+
+			spy := &spyLogger{}
+			log = spy
+			keyringEnvNotice = tt.notice
+			verbose = false
+
+			cmd := newRootCommand(func(*cobra.Command, []string) error { return nil })
+			cmd.SetOut(io.Discard)
+			cmd.SetErr(io.Discard)
+			cmd.SetArgs(tt.args)
+
+			if err := cmd.Execute(); err != nil {
+				t.Fatalf("Execute() error = %v", err)
+			}
+
+			var found bool
+			for _, entry := range spy.messages {
+				if strings.Contains(entry, "keyring") {
+					found = true
+				}
+			}
+			if found != tt.wantLogs {
+				t.Errorf("notice logged = %v, want %v (calls: %v)", found, tt.wantLogs, spy.messages)
+			}
+		})
+	}
+}

cmd/root.go

@@ -14,6 +14,7 @@ import (
 	survey "github.com/Iilun/survey/v2"
 	"github.com/aaearon/grant-cli/internal/cache"
 	"github.com/aaearon/grant-cli/internal/config"
+	"github.com/aaearon/grant-cli/internal/keyringenv"
 	"github.com/aaearon/grant-cli/internal/sca"
 	"github.com/aaearon/grant-cli/internal/sca/models"
 	"github.com/aaearon/grant-cli/internal/ui"
@@ -39,6 +40,18 @@ var verbose bool
 // so this stays false — allowing Execute() to suppress the verbose hint.
 var passedArgValidation bool
 
+// keyringApply forces the SDK's file-based keyring backend when running under
+// WSL. Injectable for tests.
+var keyringApply = keyringenv.Apply
+
+// keyringEnvNotice holds the diagnostic produced when the keyring override is
+// applied. It is stashed at startup (before --verbose is parsed) and emitted
+// from PersistentPreRunE once the flag is known.
+//
+// It is set, never cleared: the binary applies the override exactly once per
+// process. Tests that invoke the wrapper repeatedly must reset it themselves.
+var keyringEnvNotice string
+
 // elevateFlags holds the command-line flags for elevation
 type elevateFlags struct {
 	provider string
@@ -96,6 +109,9 @@ Examples:
 			passedArgValidation = true
 			if verbose {
 				sdkconfig.EnableVerboseLogging("INFO")
+				if keyringEnvNotice != "" {
+					log.Info("%s", keyringEnvNotice)
+				}
 			} else {
 				sdkconfig.DisableVerboseLogging()
 			}
@@ -252,9 +268,25 @@ func NewRootCommandWithDeps(
 	})
 }
 
+// executeWithKeyringOverride applies the WSL keyring override before running
+// cmd, so no keyring access can happen against a backend that may hang.
+//
+// It fails closed: if the override cannot be applied the command never runs,
+// because continuing would walk the user into an unbounded D-Bus hang.
+func executeWithKeyringOverride(cmd *cobra.Command) error {
+	applied, reason, err := keyringApply()
+	if err != nil {
+		return fmt.Errorf("could not force the file-based keyring backend: %w (set IDSEC_BASIC_KEYRING=1 manually and retry)", err)
+	}
+	if applied {
+		keyringEnvNotice = reason
+	}
+	return cmd.Execute()
+}
+
 func Execute() {
 	passedArgValidation = false
-	if err := rootCmd.Execute(); err != nil {
+	if err := executeWithKeyringOverride(rootCmd); err != nil {
 		fmt.Fprintln(rootCmd.ErrOrStderr(), err)
 		if !verbose && passedArgValidation {
 			fmt.Fprintln(rootCmd.ErrOrStderr(), "Hint: re-run with --verbose for more details")