fix: sweep expired token files and document uninstalling

CHANGELOG.md

@@ -20,6 +20,13 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   skips the sweep when the cache dir is a symlink or, with no home dir, a path
   relative to the working directory.
 
+- **Stale access-token files stayed on disk forever.** Without a usable OS
+  keyring, c1i caches one token file per set of credentials under
+  `<UserConfigDir>/c1i/tokens/`. Writing one now deletes the others it would
+  refuse to load, such as expired or corrupt ones, and temp files over a minute
+  old. Keyring tokens can't be listed, so they aren't swept. The README now
+  lists what an uninstall leaves behind and how to remove it.
+
 - **`requests create grant --duration` documented a format the server
   refuses.** Its help and the README suggested `24h` or `7d`, which the API
   rejects with `invalid google.protobuf.Duration value` (exit 2). They now say

README.md

@@ -1312,7 +1312,10 @@ token and reuses it until it nears expiry. It uses the OS keyring when
 available. On Unix-like hosts without a usable keyring, it instead uses a
 hardened `0600` file under the config directory (`~/.config/c1i/tokens/` on
 Linux). A cached token the server rejects (clock skew, or a revoked credential)
-is dropped and re-minted once automatically.
+is dropped and re-minted once automatically. Each file-cache write also deletes
+the other token files there that c1i would refuse to load (expired or expiring
+within a minute, corrupt or unreadable, or open to group or other users) and
+temp files over a minute old.
 The token is strictly shorter-lived than the client secret already stored
 beside it. Set `C1I_NO_TOKEN_CACHE=1` to disable caching and mint per
 invocation.
@@ -1381,6 +1384,52 @@ compromised distribution origin could withhold upgrades or offer any signed
 release newer than yours, yanked or prerelease, but never an unsigned build or
 one older than the one you run.
 
+## Uninstalling
+
+Removing the binary leaves these behind:
+
+- `~/.c1i/`, the OpenAPI spec cache.
+- `~/.c1i.yaml`, the config file.
+- `<UserConfigDir>/c1i/`, holding the file-fallback credentials
+  (`credentials/`) and the access-token file cache (`tokens/`). See
+  [Credential sources](#credential-sources) for each OS's config directory.
+- OS keyring entries: credentials under `c1i/<host>`, and cached access tokens
+  under `com.conductorone.c1i.tokens`. Older releases stored a
+  `<name>.conductor.one` tenant's credentials under `c1i/<name>`.
+
+c1i can't list keyring entries or keep track of the tenants you've logged in to.
+To remove its keyring credentials, run `auth logout` once per tenant before you
+remove the binary. If `C1I_CLIENT_ID` and `C1I_CLIENT_SECRET` are both set, unset
+them first: logout still removes the stored credentials, but drops the cached
+token for the env-var credentials instead of theirs.
+
+```sh
+c1i auth logout --url example.conductor.one
+```
+
+Logout also drops the cached token for the credentials stored now. A token
+cached under credentials you since replaced stays in the keyring. To remove
+keyring entries by hand, replace `<host>` below with a tenant's host, such as
+`example.conductor.one`, or with `<name>` for an entry an older release stored:
+
+```sh
+# macOS: each call deletes one item; repeat until it reports "could not be found"
+security delete-generic-password -s 'c1i/<host>'
+security delete-generic-password -s com.conductorone.c1i.tokens
+
+# Linux (Secret Service): each call deletes every matching item
+secret-tool clear service 'c1i/<host>'
+secret-tool clear service com.conductorone.c1i.tokens
+```
+
+On Windows, open Credential Manager → Windows Credentials and remove
+`c1i/<host>:client_id`, `c1i/<host>:client_secret`, and every
+`com.conductorone.c1i.tokens:` entry. To find which hosts you have, search for
+`c1i` in Keychain Access (macOS), Seahorse (Linux) or Credential Manager.
+
+`~/.sigstore/root/`, which `upgrade` uses to verify releases, is shared with
+other Sigstore tools, so leave it in place.
+
 ## License
 
 Apache 2.0

internal/tokensource/cache.go

@@ -36,6 +36,9 @@ const (
 	maxCachedTokenBytes = 64 << 10
 
 	noCacheEnv = "C1I_NO_TOKEN_CACHE"
+
+	// tmpFileGrace spares a temp file a concurrent store may still be renaming.
+	tmpFileGrace = time.Minute
 )
 
 const tokenKeychainService = "com.conductorone.c1i.tokens"
@@ -112,11 +115,16 @@ func loadFileCachedToken(key string) *oauth2.Token {
 	if !trustedCacheDir(filepath.Dir(filepath.Dir(p))) || !trustedCacheDir(filepath.Dir(p)) {
 		return nil
 	}
+	return readCachedTokenFile(p)
+}
+
+// readCachedTokenFile assumes the caller has checked p's directories.
+func readCachedTokenFile(p string) *oauth2.Token {
 	info, err := os.Lstat(p)
 	if err != nil || !info.Mode().IsRegular() || info.Mode().Perm()&0o077 != 0 {
 		return nil
 	}
-	f, err := os.Open(p) // #nosec G304 -- p contains only a locally-derived cache key
+	f, err := os.Open(p) // #nosec G304 -- p is a local cache key or an entry of the checked cache dir
 	if err != nil {
 		return nil
 	}
@@ -187,7 +195,64 @@ func storeFileCachedToken(key string, b []byte) {
 	if err := tmp.Close(); err != nil {
 		return
 	}
-	_ = os.Rename(tmpName, p)
+	if os.Rename(tmpName, p) == nil {
+		pruneTokenDir(dir, filepath.Base(p))
+	}
+}
+
+// pruneTokenDir deletes the token files in dir that no load would serve, so a
+// token minted from credentials since replaced or removed doesn't stay on disk
+// forever. It is best-effort and touches only regular files named exactly as
+// this package names them. Keyring entries can't be enumerated, so expired
+// tokens stored there are only ever overwritten, never swept.
+func pruneTokenDir(dir, keep string) {
+	if !trustedCacheDir(filepath.Dir(dir)) || !trustedCacheDir(dir) {
+		return
+	}
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return
+	}
+	for _, e := range entries {
+		name := e.Name()
+		p := filepath.Join(dir, name)
+		info, err := os.Lstat(p)
+		if err != nil || !info.Mode().IsRegular() || name == keep {
+			continue
+		}
+		isToken, isTemp := tokenFileName(name)
+		switch {
+		case isToken:
+			// Delete corrupt or unreadable files too: no load serves them, and a transient error costs one re-mint.
+			if readCachedTokenFile(p) != nil {
+				continue
+			}
+		case isTemp:
+			if time.Since(info.ModTime()) < tmpFileGrace {
+				continue
+			}
+		default:
+			continue
+		}
+		_ = os.Remove(p)
+	}
+}
+
+// tokenFileName reports whether name is shaped like a file cachePath or
+// storeFileCachedToken's temp file would create.
+func tokenFileName(name string) (isToken, isTemp bool) {
+	if key, ok := strings.CutSuffix(name, ".json"); ok {
+		return isCacheKey(key), false
+	}
+	rest, dotted := strings.CutPrefix(name, ".")
+	key, _, found := strings.Cut(rest, ".json.tmp-")
+	return false, dotted && found && isCacheKey(key)
+}
+
+// isCacheKey matches cacheKey's output exactly, lowercase included.
+func isCacheKey(s string) bool {
+	b, err := hex.DecodeString(s)
+	return err == nil && len(b) == sha256.Size && hex.EncodeToString(b) == s
 }
 
 func invalidateFileCachedToken(key string) {

internal/tokensource/cache_test.go

@@ -444,3 +444,171 @@ func TestCacheKeyChangeForcesMint(t *testing.T) {
 		t.Errorf("token=%q mints=%d, want fresh token from exactly one mint", got.AccessToken, m.n)
 	}
 }
+
+func writeTokenFile(t *testing.T, p string, expiry time.Time) {
+	t.Helper()
+	body := `{"access_token":"t","token_type":"Bearer","expiry":"` + expiry.UTC().Format(time.RFC3339) + `"}`
+	if err := os.WriteFile(p, []byte(body), 0o600); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestStoreSweepsExpiredTokenFiles(t *testing.T) {
+	useTempConfig(t)
+	keep, _ := cachePath(testCacheKey("host", "client"))
+	dir := filepath.Dir(keep)
+	if err := os.MkdirAll(dir, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	expired := filepath.Join(dir, cacheKey("h", "c", "old-secret")+".json")
+	nearExpiry := filepath.Join(dir, cacheKey("h", "c", "skew-secret")+".json")
+	fresh := filepath.Join(dir, cacheKey("h", "c", "other-secret")+".json")
+	corrupt := filepath.Join(dir, cacheKey("h", "c", "corrupt-secret")+".json")
+	staleTmp := filepath.Join(dir, "."+cacheKey("h", "c", "old-secret")+".json.tmp-1")
+	youngTmp := filepath.Join(dir, "."+cacheKey("h", "c", "other-secret")+".json.tmp-2")
+	notOurs := filepath.Join(dir, "notes.txt")
+	key := cacheKey("h", "c", "near-miss-secret")
+	// Each is expired and old, so only its name keeps it.
+	var nearMisses []string
+	for _, name := range []string{
+		"backup.json", "abcd.json", key[:32] + ".json",
+		".backup.json.tmp-old", ".abcd.json.tmp-old", key + ".json.tmp-old", "." + key,
+		"." + key + ".json.tmpx",
+	} {
+		nearMisses = append(nearMisses, filepath.Join(dir, name))
+	}
+	subdir := filepath.Join(dir, cacheKey("h", "c", "dir-secret")+".json")
+	upperJSON := filepath.Join(dir, strings.ToUpper(cacheKey("h", "c", "upper-secret"))+".json")
+	loosePerms := filepath.Join(dir, cacheKey("h", "c", "perm-secret")+".json")
+	oversized := filepath.Join(dir, cacheKey("h", "c", "big-secret")+".json")
+	if err := os.Mkdir(subdir, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	writeTokenFile(t, expired, time.Now().Add(-time.Hour))
+	writeTokenFile(t, nearExpiry, time.Now().Add(30*time.Second))
+	writeTokenFile(t, fresh, time.Now().Add(time.Hour))
+	writeTokenFile(t, upperJSON, time.Now().Add(-time.Hour))
+	writeTokenFile(t, loosePerms, time.Now().Add(time.Hour))
+	if err := os.Chmod(loosePerms, 0o644); err != nil {
+		t.Fatal(err)
+	}
+	// Valid JSON once the padding is trimmed, so only the size bound rejects it.
+	writeTokenFile(t, oversized, time.Now().Add(time.Hour))
+	f, err := os.OpenFile(oversized, os.O_APPEND|os.O_WRONLY, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := f.WriteString(strings.Repeat(" ", maxCachedTokenBytes)); err != nil {
+		t.Fatal(err)
+	}
+	if err := f.Close(); err != nil {
+		t.Fatal(err)
+	}
+	// A link is never followed or removed, even when its target is an expired token.
+	link := filepath.Join(dir, cacheKey("h", "c", "link-secret")+".json")
+	target := filepath.Join(t.TempDir(), "target.json")
+	writeTokenFile(t, target, time.Now().Add(-time.Hour))
+	survivors := append([]string{keep, fresh, youngTmp, notOurs, subdir, upperJSON}, nearMisses...)
+	if err := os.Symlink(target, link); err == nil {
+		survivors = append(survivors, link, target)
+	}
+	for _, p := range []string{corrupt, staleTmp, youngTmp, notOurs} {
+		if err := os.WriteFile(p, []byte("not json"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	old := time.Now().Add(-2 * tmpFileGrace)
+	for _, p := range nearMisses {
+		writeTokenFile(t, p, time.Now().Add(-time.Hour))
+	}
+	for _, p := range append([]string{staleTmp}, nearMisses...) {
+		if err := os.Chtimes(p, old, old); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	storeCachedToken(testCacheKey("host", "client"), freshToken(30*time.Minute))
+
+	for _, p := range []string{expired, nearExpiry, corrupt, staleTmp, loosePerms, oversized} {
+		if _, err := os.Lstat(p); !errors.Is(err, os.ErrNotExist) {
+			t.Errorf("%s survived the sweep: %v", filepath.Base(p), err)
+		}
+	}
+	for _, p := range survivors {
+		if _, err := os.Lstat(p); err != nil {
+			t.Errorf("%s was removed: %v", filepath.Base(p), err)
+		}
+	}
+}
+
+// The token just written is kept even when it is already too close to expiry
+// for a load to serve it.
+func TestStoreSweepKeepsTokenJustWritten(t *testing.T) {
+	useTempConfig(t)
+	key := testCacheKey("host", "client")
+	storeCachedToken(key, freshToken(30*time.Second))
+	p, _ := cachePath(key)
+	if _, err := os.Lstat(p); err != nil {
+		t.Errorf("token just written was swept: %v", err)
+	}
+}
+
+func TestStoreSweepLeavesCredentialsDir(t *testing.T) {
+	dir := useTempConfig(t)
+	creds := filepath.Join(dir, "c1i", "credentials")
+	if err := os.MkdirAll(creds, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	cred := filepath.Join(creds, cacheKey("h", "c", "cred-secret")+".json")
+	writeTokenFile(t, cred, time.Now().Add(-time.Hour))
+
+	key := testCacheKey("host", "client")
+	storeCachedToken(key, freshToken(30*time.Minute))
+
+	if p, _ := cachePath(key); loadFileCachedToken(key) == nil {
+		t.Fatalf("store did not write %s, so the sweep never ran", p)
+	}
+	if _, err := os.Lstat(cred); err != nil {
+		t.Errorf("credentials file was removed: %v", err)
+	}
+}
+
+// Store refuses a symlinked tokens dir or c1i parent before the sweep runs, so
+// this calls the sweep directly to hold its own guard.
+func TestTokenSweepSkipsSymlinkedDir(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("symlink privileges are not portable on Windows")
+	}
+	for _, linked := range []string{"tokens", "c1i"} {
+		t.Run(linked, func(t *testing.T) {
+			dir := useTempConfig(t)
+			target := filepath.Join(dir, "untrusted")
+			tokens := filepath.Join(dir, "c1i", "tokens")
+			strayDir := target
+			if linked == "c1i" {
+				strayDir = filepath.Join(target, "tokens")
+			}
+			if err := os.MkdirAll(strayDir, 0o700); err != nil {
+				t.Fatal(err)
+			}
+			stray := filepath.Join(strayDir, cacheKey("h", "c", "stray-secret")+".json")
+			writeTokenFile(t, stray, time.Now().Add(-time.Hour))
+			link := filepath.Join(dir, "c1i")
+			if linked == "tokens" {
+				link = tokens
+				if err := os.MkdirAll(filepath.Dir(tokens), 0o700); err != nil {
+					t.Fatal(err)
+				}
+			}
+			if err := os.Symlink(target, link); err != nil {
+				t.Skipf("creating symlink: %v", err)
+			}
+
+			pruneTokenDir(tokens, "")
+
+			if _, err := os.Lstat(stray); err != nil {
+				t.Errorf("file behind a symlinked %s dir was removed: %v", linked, err)
+			}
+		})
+	}
+}