From 49c5336992920d04e58871df75041f8c4b86a548 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 21 May 2026 13:05:41 -0400 Subject: [PATCH 1/2] fix(config): clear --all removes legacy config file, not just legacy cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the LegacyCacheDir both-dir cleanup pattern on the config side. After MON-5371 relocated the config dir from ~/.config/google-readonly to the OS-native dir on macOS/Windows, clear --all only removed the new-location config.yml; the legacy file then re-armed the DetectConfigRelocation divergence trap on next gro init. Adds: - config.OldHandRolledConfigDir exported primitive (symmetric with OldHandRolledTokenPath, LegacyCacheDir). - config.GetConfigDirNoCreate (symmetric with GetConfigPathNoCreate) so the candidate helper resolves without side-effects. - internal/cmd/config.configFilesForClear: per-file deduped candidate list covering both canonical config.{yml,json} and the legacy hand- rolled dir's config.{yml,json}. configFilesForClearFn is a package- var test seam so Linux CI can exercise the macOS/Windows divergent- dir branch. - runClear hardening (§6.6 pattern 7): under --all an OpenNoMigrate failure (e.g. invalid keyring.backend) soft-degrades with a warning so the file scrub still completes. Plain clear (no --all) still hard-fails on a broken keyring. OAuth client JSON is intentionally NOT removed (deployment material exclusion preserved). Closes #137 [MON-5433] --- internal/cmd/config/clear_legacy_test.go | 215 +++++++++++++++++++++++ internal/cmd/config/config.go | 112 +++++++++--- internal/config/config.go | 7 + internal/config/relocate.go | 9 + 4 files changed, 319 insertions(+), 24 deletions(-) create mode 100644 internal/cmd/config/clear_legacy_test.go diff --git a/internal/cmd/config/clear_legacy_test.go b/internal/cmd/config/clear_legacy_test.go new file mode 100644 index 0000000..03da570 --- /dev/null +++ b/internal/cmd/config/clear_legacy_test.go @@ -0,0 +1,215 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/open-cli-collective/google-readonly/internal/credtest" +) + +// withSyntheticConfigCandidates overrides configFilesForClearFn so Linux CI +// can exercise the macOS/Windows "old != new" branch with explicit paths. +// Same shape as MON-5373 nrq's credentialFileCandidates seam. +func withSyntheticConfigCandidates(t *testing.T, paths []string) { + t.Helper() + orig := configFilesForClearFn + configFilesForClearFn = func() ([]string, error) { return paths, nil } + t.Cleanup(func() { configFilesForClearFn = orig }) +} + +func seedFile(t *testing.T, path, contents string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("MkdirAll %s: %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatalf("WriteFile %s: %v", path, err) + } +} + +func TestConfigClearAll_RemovesAllConfigCandidates(t *testing.T) { + credtest.Setup(t) + tmp := t.TempDir() + newYML := filepath.Join(tmp, "new", "config.yml") + newJSON := filepath.Join(tmp, "new", "config.json") + oldYML := filepath.Join(tmp, "old", "config.yml") + oldJSON := filepath.Join(tmp, "old", "config.json") + for _, p := range []string{newYML, newJSON, oldYML, oldJSON} { + seedFile(t, p, "credential_ref: foo/bar\n") + } + withSyntheticConfigCandidates(t, []string{newYML, newJSON, oldYML, oldJSON}) + + _ = capture(t, func() { + if err := runClear(true, false); err != nil { + t.Fatalf("runClear: %v", err) + } + }) + + for _, p := range []string{newYML, newJSON, oldYML, oldJSON} { + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Errorf("--all must remove %s (stat err=%v)", p, err) + } + } +} + +func TestConfigClearAll_DryRunReportsAllCandidates(t *testing.T) { + credtest.Setup(t) + tmp := t.TempDir() + newYML := filepath.Join(tmp, "new", "config.yml") + oldJSON := filepath.Join(tmp, "old", "config.json") + seedFile(t, newYML, "credential_ref: foo/bar\n") + seedFile(t, oldJSON, `{"credential_ref":"legacy/bar"}`) + withSyntheticConfigCandidates(t, []string{newYML, oldJSON}) + + out := capture(t, func() { + if err := runClear(true, true); err != nil { + t.Fatalf("runClear dry-run: %v", err) + } + }) + + for _, p := range []string{newYML, oldJSON} { + if !strings.Contains(out, p) && !strings.Contains(out, shortened(t, p)) { + t.Errorf("--dry-run --all should name %q in output, got:\n%s", p, out) + } + if _, err := os.Stat(p); err != nil { + t.Errorf("--dry-run must not remove %s (stat err=%v)", p, err) + } + } +} + +func TestConfigClearAll_LegacyAbsent_OnlyCanonical(t *testing.T) { + credtest.Setup(t) + tmp := t.TempDir() + newYML := filepath.Join(tmp, "new", "config.yml") + oldYML := filepath.Join(tmp, "old", "config.yml") // never seeded + seedFile(t, newYML, "credential_ref: foo/bar\n") + withSyntheticConfigCandidates(t, []string{newYML, oldYML}) + + out := capture(t, func() { + if err := runClear(true, false); err != nil { + t.Fatalf("runClear: %v", err) + } + }) + + if _, err := os.Stat(newYML); !os.IsNotExist(err) { + t.Errorf("--all must remove canonical %s (stat err=%v)", newYML, err) + } + // Absent legacy must not produce a "Removed" line. We accept silence on + // IsNotExist. + if strings.Contains(out, oldYML) { + t.Errorf("absent %s should not appear in --all output, got:\n%s", oldYML, out) + } +} + +func TestConfigFilesForClear_PathIdentityDedup(t *testing.T) { + credtest.Setup(t) + // Engineer oldDir == newDir by pointing XDG_CONFIG_HOME at the same root + // the canonical resolver uses. credtest.Setup put us under a hermetic + // statedirtest env; on Linux that already produces old==new — the test + // is meaningful there. On macOS/Windows the resolvers diverge by design + // and this assertion just verifies no spurious duplication when they + // happen to coincide (the canonical paths are themselves distinct). + paths, err := configFilesForClear() + if err != nil { + t.Fatalf("configFilesForClear: %v", err) + } + seen := map[string]int{} + for _, p := range paths { + seen[p]++ + } + for p, n := range seen { + if n != 1 { + t.Errorf("path %s appeared %d times; want 1 (per-file dedup)", p, n) + } + } + // Both forms must always be present at the canonical dir (regression pin + // for the Codex r2 minor: canonical config.json was originally missed). + newDir, derr := newDirFor(t) + if derr != nil { + t.Fatalf("newDirFor: %v", derr) + } + wantYML := filepath.Join(newDir, "config.yml") + wantJSON := filepath.Join(newDir, "config.json") + if seen[wantYML] == 0 { + t.Errorf("candidate list missing canonical config.yml at %s; got %v", wantYML, paths) + } + if seen[wantJSON] == 0 { + t.Errorf("candidate list missing canonical config.json at %s; got %v", wantJSON, paths) + } +} + +func TestConfigClearAll_MalformedCanonicalConfig_StillScrubsFiles(t *testing.T) { + credtest.Setup(t) + tmp := t.TempDir() + newYML := filepath.Join(tmp, "new", "config.yml") + // File contents are irrelevant for scrub-completion; the failure mode + // being pinned is "keyring open fails / config malformed → file scrub + // still completes". We force the keyring side via an unknown backend + // env value, which credstore.Open() must reject. + seedFile(t, newYML, "credential_ref: foo/bar\n") + t.Setenv("GOOGLE_READONLY_KEYRING_BACKEND", "this-backend-does-not-exist") + withSyntheticConfigCandidates(t, []string{newYML}) + + out := capture(t, func() { + // We expect runClear to NOT return an error under --all even though + // the keyring open fails — that is the recovery contract. + if err := runClear(true, false); err != nil { + t.Fatalf("runClear under --all must soft-degrade on keyring failure, got: %v", err) + } + }) + + if _, err := os.Stat(newYML); !os.IsNotExist(err) { + t.Errorf("--all must scrub %s even when keyring open fails (stat err=%v)", newYML, err) + } + // We don't assert exact warning wording, only that a Removed line was + // emitted for the candidate (cheap visibility into "did we actually + // reach the scrub"). + if !strings.Contains(out, newYML) && !strings.Contains(out, shortened(t, newYML)) { + t.Errorf("expected runClear to print Removed line for %s, got:\n%s", newYML, out) + } +} + +func TestConfigClear_PlainStillHardFailsOnBrokenKeyring(t *testing.T) { + credtest.Setup(t) + t.Setenv("GOOGLE_READONLY_KEYRING_BACKEND", "this-backend-does-not-exist") + + _ = capture(t, func() { + err := runClear(false, false) + if err == nil { + t.Fatal("plain `clear` (no --all) must surface a keyring open failure; got nil") + } + }) +} + +// shortened returns config.ShortenPath(p) via a callback-style indirection +// that avoids importing internal/config in the test file just for one call. +// Production code already prints the shortened form, so the test must match +// whichever form appears. +func shortened(t *testing.T, p string) string { + t.Helper() + home, err := os.UserHomeDir() + if err != nil { + return p + } + if strings.HasPrefix(p, home) { + return "~" + strings.TrimPrefix(p, home) + } + return p +} + +// newDirFor returns the canonical config dir as configFilesForClear would +// resolve it (via the live config.GetConfigDirNoCreate). Kept as a helper +// so tests don't have to import internal/config solely for this. +func newDirFor(t *testing.T) (string, error) { + t.Helper() + paths, err := configFilesForClear() + if err != nil { + return "", err + } + if len(paths) == 0 { + t.Fatal("configFilesForClear returned no paths") + } + return filepath.Dir(paths[0]), nil +} diff --git a/internal/cmd/config/config.go b/internal/cmd/config/config.go index 1f7b825..aa98fe0 100644 --- a/internal/cmd/config/config.go +++ b/internal/cmd/config/config.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "github.com/spf13/cobra" @@ -18,6 +19,45 @@ import ( "github.com/open-cli-collective/google-readonly/internal/output" ) +// configFilesForClear returns the config files `clear --all` should remove, +// deduped by path-identity (Linux collapses old==new). Symmetric with the +// cache side's CacheDirPath + LegacyCacheDir treatment. Both forms +// (config.yml and the legacy config.json) are listed at both the canonical +// and old hand-rolled dirs because LoadConfig still reads new-dir +// config.json when YAML is absent, and the relocation gate can still copy +// either form forward from the old dir. The OAuth client JSON is +// intentionally excluded (deployment material, per the existing trailing +// note in runClear). Resolves WITHOUT creating directories. +func configFilesForClear() ([]string, error) { + newDir, err := config.GetConfigDirNoCreate() + if err != nil { + return nil, err + } + oldDir, _ := config.OldHandRolledConfigDir() // best-effort: no HOME → skip legacy + + var paths []string + seen := map[string]struct{}{} + add := func(p string) { + if _, dup := seen[p]; dup { + return + } + seen[p] = struct{}{} + paths = append(paths, p) + } + add(filepath.Join(newDir, config.ConfigFileYAML)) + add(filepath.Join(newDir, config.ConfigFile)) + if oldDir != "" { + add(filepath.Join(oldDir, config.ConfigFileYAML)) + add(filepath.Join(oldDir, config.ConfigFile)) + } + return paths, nil +} + +// configFilesForClearFn is the package-var test seam: tests inject a +// synthetic distinct-old/new path list so Linux CI can exercise the +// macOS/Windows "old != new" branch without OS-specific paths. +var configFilesForClearFn = configFilesForClear + // NewCommand returns the config command with subcommands. func NewCommand() *cobra.Command { cmd := &cobra.Command{ @@ -210,33 +250,52 @@ func runTest(cmd *cobra.Command, _ []string) error { } func runClear(all, dryRun bool) error { - // OpenNoMigrate: clear is a §1.8 remediation path ("clear the conflicting - // entry, then re-run") — running migration first would block it. - st, err := keychain.OpenNoMigrate() + // Resolve scrub targets BEFORE opening the keyring (§6.6 pattern 7): + // `--all` is the user's primary recovery path and must not itself be + // blocked by the broken state it exists to wipe. Path resolution is + // side-effect-free; failure here is fatal regardless of --all. + cfgPaths, err := configFilesForClearFn() if err != nil { - return err + return fmt.Errorf("resolving config paths: %w", err) } - defer func() { _ = st.Close() }() - hasTok, err := st.HasToken() + // OpenNoMigrate: clear is a §1.8 remediation path ("clear the conflicting + // entry, then re-run") — running migration first would block it. Under + // --all an open failure (e.g. invalid keyring.backend in a malformed + // canonical config) is soft-degraded so the file scrub still runs. + st, err := keychain.OpenNoMigrate() if err != nil { - return fmt.Errorf("checking stored token: %w", err) + if !all { + return err + } + fmt.Fprintf(os.Stderr, "warning: could not open keyring (%v) — proceeding with file scrub only\n", err) + st = nil } - // NoCreate: clear (incl. --dry-run) must not create the config dir as a - // side-effect. os.Remove below tolerates an absent dir/file. - cfgPath, err := config.GetConfigPathNoCreate() - if err != nil { - return fmt.Errorf("resolving config path: %w", err) + if st != nil { + defer func() { _ = st.Close() }() + } + + hasTok := false + if st != nil { + hasTok, err = st.HasToken() + if err != nil { + return fmt.Errorf("checking stored token: %w", err) + } } if dryRun { - if hasTok { + switch { + case st == nil: + fmt.Println("Would remove: (keyring unavailable — token state unknown)") + case hasTok: fmt.Printf("Would remove: OAuth token at %s\n", st.Ref()) - } else { + default: fmt.Println("Would remove: (no OAuth token present)") } if all { - fmt.Printf("Would remove: %s\n", config.ShortenPath(cfgPath)) + for _, p := range cfgPaths { + fmt.Printf("Would remove: %s\n", config.ShortenPath(p)) + } // Non-creating resolver: --dry-run must not create or migrate. if cacheDir, cerr := config.CacheDirPath(); cerr == nil { fmt.Printf("Would remove: Drive metadata cache at %s\n", config.ShortenPath(cacheDir)) @@ -247,23 +306,28 @@ func runClear(all, dryRun bool) error { return nil } - if hasTok { + switch { + case st == nil: + // Already warned above; skip token cleanup. + case hasTok: if err := st.DeleteToken(); err != nil { return fmt.Errorf("clearing token: %w", err) } fmt.Printf("Cleared OAuth token from %s.\n", st.Ref()) - } else { + default: fmt.Println("No OAuth token found to clear.") } if all { - switch err := os.Remove(cfgPath); { - case err == nil: - fmt.Printf("Removed %s.\n", config.ShortenPath(cfgPath)) - case os.IsNotExist(err): - fmt.Printf("No %s to remove.\n", config.ShortenPath(cfgPath)) - default: - return fmt.Errorf("removing %s: %w", config.ShortenPath(cfgPath), err) + for _, p := range cfgPaths { + switch err := os.Remove(p); { + case err == nil: + fmt.Printf("Removed %s.\n", config.ShortenPath(p)) + case os.IsNotExist(err): + // not present — fine + default: + return fmt.Errorf("removing %s: %w", config.ShortenPath(p), err) + } } // Drive metadata cache: an explicit full reset removes BOTH the diff --git a/internal/config/config.go b/internal/config/config.go index e9e85db..a840e44 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -117,6 +117,13 @@ func GetConfigDir() (string, error) { return configScope.ConfigDirEnsured() } +// GetConfigDirNoCreate resolves the configuration directory WITHOUT creating +// it — for side-effect-free callers such as `config clear --all` (incl. +// `--dry-run`). Symmetric with GetConfigPathNoCreate. +func GetConfigDirNoCreate() (string, error) { + return configDirPath() +} + // CacheDirPath resolves the OS-designated cache directory WITHOUT creating it // (used by `config clear --all --dry-run` and tests). os.UserCacheDir gives // the canonical per-OS root: Linux $XDG_CACHE_HOME or ~/.cache, macOS diff --git a/internal/config/relocate.go b/internal/config/relocate.go index c4f0006..48d2a35 100644 --- a/internal/config/relocate.go +++ b/internal/config/relocate.go @@ -60,6 +60,15 @@ func oldHandRolledConfigDir() (string, error) { return filepath.Join(configHome, DirName), nil } +// OldHandRolledConfigDir is the pre-MON-5371 hand-rolled config directory +// (~/.config/google-readonly on macOS/Windows; identical to the canonical +// dir on Linux). Exposed so cleanup commands can scrub legacy config files +// at this location — symmetric with LegacyCacheDir on the cache side. +// Non-creating. Returns ("", error) if HOME is unresolvable. +func OldHandRolledConfigDir() (string, error) { + return oldHandRolledConfigDir() +} + // OldHandRolledTokenPath is the pre-MON-5371 legacy token.json location. // Exported so keychain.migrate's token-source enumeration can probe it with // full §1.8 conflict semantics (per the MON-5371 plan: token.json is excluded From 3d4989abee35bc23196292a66e8e6a369ad1c200 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 21 May 2026 13:24:04 -0400 Subject: [PATCH 2/2] fix(config): soft-degrade HasToken failure under --all; honest test names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex/TDD PR review caught two issues: 1. (Major) runClear's --all soft-degrade only covered OpenNoMigrate failure; HasToken failure still short-circuited the file scrub. Symmetric soft-degrade now applied — emits a warning, closes the store eagerly, drops the reference so token branches skip cleanly. Defer is nil-guarded since st may be set to nil mid-function. 2. (Minor) The "MalformedCanonicalConfig" test seeded valid YAML and forced failure via the keyring backend env — overclaiming. Split into two honest tests: BrokenKeyringOpen (forces OpenNoMigrate failure) and MalformedCanonicalConfig (genuinely malformed YAML on disk; pins that runClear no longer reads the config). Added ExcludesOAuthClientJSON to pin the deployment-material exclusion. Also switched the test file to use appconfig.ShortenPath directly instead of a parallel implementation (Codex nit). --- internal/cmd/config/clear_legacy_test.go | 75 +++++++++++++++--------- internal/cmd/config/config.go | 27 +++++++-- 2 files changed, 67 insertions(+), 35 deletions(-) diff --git a/internal/cmd/config/clear_legacy_test.go b/internal/cmd/config/clear_legacy_test.go index 03da570..6399f83 100644 --- a/internal/cmd/config/clear_legacy_test.go +++ b/internal/cmd/config/clear_legacy_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + appconfig "github.com/open-cli-collective/google-readonly/internal/config" "github.com/open-cli-collective/google-readonly/internal/credtest" ) @@ -70,7 +71,7 @@ func TestConfigClearAll_DryRunReportsAllCandidates(t *testing.T) { }) for _, p := range []string{newYML, oldJSON} { - if !strings.Contains(out, p) && !strings.Contains(out, shortened(t, p)) { + if !strings.Contains(out, p) && !strings.Contains(out, appconfig.ShortenPath(p)) { t.Errorf("--dry-run --all should name %q in output, got:\n%s", p, out) } if _, err := os.Stat(p); err != nil { @@ -140,63 +141,79 @@ func TestConfigFilesForClear_PathIdentityDedup(t *testing.T) { } } -func TestConfigClearAll_MalformedCanonicalConfig_StillScrubsFiles(t *testing.T) { +func TestConfigClearAll_BrokenKeyringOpen_StillScrubsFiles(t *testing.T) { credtest.Setup(t) tmp := t.TempDir() newYML := filepath.Join(tmp, "new", "config.yml") - // File contents are irrelevant for scrub-completion; the failure mode - // being pinned is "keyring open fails / config malformed → file scrub - // still completes". We force the keyring side via an unknown backend - // env value, which credstore.Open() must reject. + // Force keychain.OpenNoMigrate to fail via an unknown backend value. seedFile(t, newYML, "credential_ref: foo/bar\n") t.Setenv("GOOGLE_READONLY_KEYRING_BACKEND", "this-backend-does-not-exist") withSyntheticConfigCandidates(t, []string{newYML}) out := capture(t, func() { - // We expect runClear to NOT return an error under --all even though - // the keyring open fails — that is the recovery contract. if err := runClear(true, false); err != nil { - t.Fatalf("runClear under --all must soft-degrade on keyring failure, got: %v", err) + t.Fatalf("runClear under --all must soft-degrade on keyring open failure, got: %v", err) } }) if _, err := os.Stat(newYML); !os.IsNotExist(err) { t.Errorf("--all must scrub %s even when keyring open fails (stat err=%v)", newYML, err) } - // We don't assert exact warning wording, only that a Removed line was - // emitted for the candidate (cheap visibility into "did we actually - // reach the scrub"). - if !strings.Contains(out, newYML) && !strings.Contains(out, shortened(t, newYML)) { + if !strings.Contains(out, newYML) && !strings.Contains(out, appconfig.ShortenPath(newYML)) { t.Errorf("expected runClear to print Removed line for %s, got:\n%s", newYML, out) } } -func TestConfigClear_PlainStillHardFailsOnBrokenKeyring(t *testing.T) { +// TestConfigClearAll_MalformedCanonicalConfig_StillScrubsFiles pins that a +// genuinely malformed config.yml on disk does NOT block --all. The fix +// removed LoadConfig from the runClear path entirely (paths come from the +// candidate helper, not from a parsed Config), so this should be trivially +// true — the test pins it against regression. +func TestConfigClearAll_MalformedCanonicalConfig_StillScrubsFiles(t *testing.T) { credtest.Setup(t) - t.Setenv("GOOGLE_READONLY_KEYRING_BACKEND", "this-backend-does-not-exist") + tmp := t.TempDir() + newYML := filepath.Join(tmp, "new", "config.yml") + // Genuinely malformed YAML (matches the relocate_test.go fixture shape). + seedFile(t, newYML, "[unclosed_array: yes\n") + withSyntheticConfigCandidates(t, []string{newYML}) _ = capture(t, func() { - err := runClear(false, false) - if err == nil { - t.Fatal("plain `clear` (no --all) must surface a keyring open failure; got nil") + if err := runClear(true, false); err != nil { + t.Fatalf("--all must tolerate malformed canonical YAML, got: %v", err) } }) + + if _, err := os.Stat(newYML); !os.IsNotExist(err) { + t.Errorf("--all must scrub malformed %s (stat err=%v)", newYML, err) + } } -// shortened returns config.ShortenPath(p) via a callback-style indirection -// that avoids importing internal/config in the test file just for one call. -// Production code already prints the shortened form, so the test must match -// whichever form appears. -func shortened(t *testing.T, p string) string { - t.Helper() - home, err := os.UserHomeDir() +// TestConfigFilesForClear_ExcludesOAuthClientJSON pins the deployment- +// material exclusion: clear --all must never list oauth_client.json +// among the files it removes. +func TestConfigFilesForClear_ExcludesOAuthClientJSON(t *testing.T) { + credtest.Setup(t) + paths, err := configFilesForClear() if err != nil { - return p + t.Fatalf("configFilesForClear: %v", err) } - if strings.HasPrefix(p, home) { - return "~" + strings.TrimPrefix(p, home) + for _, p := range paths { + if filepath.Base(p) == appconfig.OAuthClientFile { + t.Errorf("clear --all must NOT remove the OAuth client JSON (deployment material); got %s in %v", p, paths) + } } - return p +} + +func TestConfigClear_PlainStillHardFailsOnBrokenKeyring(t *testing.T) { + credtest.Setup(t) + t.Setenv("GOOGLE_READONLY_KEYRING_BACKEND", "this-backend-does-not-exist") + + _ = capture(t, func() { + err := runClear(false, false) + if err == nil { + t.Fatal("plain `clear` (no --all) must surface a keyring open failure; got nil") + } + }) } // newDirFor returns the canonical config dir as configFilesForClear would diff --git a/internal/cmd/config/config.go b/internal/cmd/config/config.go index aa98fe0..27c8314 100644 --- a/internal/cmd/config/config.go +++ b/internal/cmd/config/config.go @@ -271,15 +271,30 @@ func runClear(all, dryRun bool) error { fmt.Fprintf(os.Stderr, "warning: could not open keyring (%v) — proceeding with file scrub only\n", err) st = nil } - if st != nil { - defer func() { _ = st.Close() }() - } + // Nil-guard the deferred close: a later HasToken soft-degrade may set + // st = nil after closing it eagerly. Closing nil would panic. + defer func() { + if st != nil { + _ = st.Close() + } + }() hasTok := false if st != nil { - hasTok, err = st.HasToken() - if err != nil { - return fmt.Errorf("checking stored token: %w", err) + h, herr := st.HasToken() + switch { + case herr == nil: + hasTok = h + case all: + // Symmetric with the OpenNoMigrate soft-degrade: a HasToken + // failure (e.g. partial backend corruption) under --all must + // not block the file scrub. Surface a warning, drop the + // store reference so token branches below skip cleanly. + fmt.Fprintf(os.Stderr, "warning: could not check stored token (%v) — proceeding with file scrub only\n", herr) + _ = st.Close() + st = nil + default: + return fmt.Errorf("checking stored token: %w", herr) } }