diff --git a/CHANGELOG.md b/CHANGELOG.md index 3163bad..4e219e3 100644 --- a/CHANGELOG.md +++ b/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 + `/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 diff --git a/README.md b/README.md index 1e4b148..5d8d656 100644 --- a/README.md +++ b/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. +- `/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/`, and cached access tokens + under `com.conductorone.c1i.tokens`. Older releases stored a + `.conductor.one` tenant's credentials under `c1i/`. + +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 `` below with a tenant's host, such as +`example.conductor.one`, or with `` 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/' +security delete-generic-password -s com.conductorone.c1i.tokens + +# Linux (Secret Service): each call deletes every matching item +secret-tool clear service 'c1i/' +secret-tool clear service com.conductorone.c1i.tokens +``` + +On Windows, open Credential Manager → Windows Credentials and remove +`c1i/:client_id`, `c1i/: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 diff --git a/internal/tokensource/cache.go b/internal/tokensource/cache.go index 08665f4..eae785a 100644 --- a/internal/tokensource/cache.go +++ b/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) { diff --git a/internal/tokensource/cache_test.go b/internal/tokensource/cache_test.go index 87e3c23..7ac9054 100644 --- a/internal/tokensource/cache_test.go +++ b/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) + } + }) + } +}