From d55de0ad0ba2f259d361962c3320d72ba039fc76 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 17 Jun 2026 09:01:13 -0400 Subject: [PATCH 1/2] fix(keyring): actionable error + documented bypass when keyring is unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runtime token resolution opens the OS keyring on every API command via keyring.ResolveToken -> Open(). On a machine where the keyring is locked, unreachable, or being driven headless under an agent (the OS unlock prompt never surfaces), the command failed with an opaque backend error and the unhelpful "run init" hint -- even for a user whose config file was already populated. There was no signposted way to run keyring-free. This wraps genuine keyring open/read failures in ResolveToken with an actionable message that names the existing, standard-sanctioned escape hatches, in resolution order: - the per-tool / shared API-token env vars (JIRA_API_TOKEN / CFL_API_TOKEN / ATLASSIAN_API_TOKEN), which are resolved BEFORE the keyring is ever opened, so the keyring is never touched; - the encrypted-file backend (--backend file) + its passphrase env var, and the pass backend (--backend pass). The original error is preserved via %w so errors.Is classification (e.g. credstore.ErrSecretServiceFailClosed / ErrBackendNotImplemented) still works. The §1.8 corrupt-shared-config graceful-degradation path is untouched: when the keyring itself is healthy it still resolves with no error. No new plaintext path is introduced -- the env var and file backend are the Secret-Handling Standard's own §1.4 fallbacks, merely surfaced so users discover them instead of hitting a silent prompt. Also documents these keyring-free options in the README so the escape hatch is discoverable before a user hits the failure. Tests cover both paths: keyring-open failure -> wrapped, actionable, underlying error preserved; env token set -> keyring fully bypassed; plus a regression guard that a corrupt shared config with a working keyring is NOT wrapped. Closes #384 --- README.md | 34 ++++++++ shared/keyring/resolve.go | 58 +++++++++++-- shared/keyring/resolve_test.go | 144 +++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 6 deletions(-) create mode 100644 shared/keyring/resolve_test.go diff --git a/README.md b/README.md index dc08bdcd..f1bf586c 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,40 @@ jtk issues list --project PROJ cfl page list --space DEV ``` +### Headless and Keyring-Free Environments + +The API token lives in the OS keyring (macOS Keychain, Windows Credential +Manager, Linux Secret Service). On a headless host, in CI, or under an +automation agent there may be no keyring — or one that is locked and +cannot be unlocked interactively (the unlock prompt never reaches you). +Both tools support keyring-free operation without putting the token in a +plaintext config file: + +- **Supply the token via an environment variable.** `ATLASSIAN_API_TOKEN` + (or the tool-specific `JIRA_API_TOKEN` / `CFL_API_TOKEN`) is resolved + *before* the keyring is ever opened, so the keyring is never touched. This + is the canonical answer for `op run`-style setups and agent automation. + + ```bash + export ATLASSIAN_API_TOKEN="your-api-token" + jtk issues list --project PROJ # never touches the OS keyring + ``` + +- **Use the encrypted-file backend.** `--backend file` (or + `ATLASSIAN_CLI_KEYRING_BACKEND=file`, or `keyring.backend: file` in the + config file) stores the token in an encrypted file instead of the OS + keyring. Supply its passphrase non-interactively with + `ATLASSIAN_CLI_KEYRING_PASSPHRASE` for unattended runs. + +- **Use the `pass` backend.** `--backend pass` (or + `ATLASSIAN_CLI_KEYRING_BACKEND=pass`) integrates with an existing + [`pass`](https://www.passwordstore.org/) password-store — the canonical + headless-Linux option when you already run one. + +If a command fails because the keyring is unavailable or locked, the error +names these same options. Run `jtk config show` / `cfl config show` to see +which backend is active on the current machine. + ### Output Representations Both tools support three output representations: diff --git a/shared/keyring/resolve.go b/shared/keyring/resolve.go index 4cb96b85..5279fc31 100644 --- a/shared/keyring/resolve.go +++ b/shared/keyring/resolve.go @@ -86,7 +86,12 @@ func envToken(tool string) (string, bool) { // the one-time §1.8 migration (Open) and the effective key is read. Env // winning does not force a keyring open — the migration then runs on the // next invocation that does open it (opportunistic, template-consistent). -// Keyring errors propagate (never folded into "absent"). +// Keyring errors propagate (never folded into "absent"), but they are +// wrapped with the headless/no-keyring escape hatches (see +// keyringUnavailableError) so a user on a machine without a usable OS +// keyring — or running the CLI headless under an agent, where the unlock +// prompt never surfaces (issue #384) — gets an actionable, self-service +// message instead of an opaque backend error. func ResolveToken(tool string) (string, TokenSource, error) { if v, ok := envToken(tool); ok { return v, SourceEnv, nil @@ -96,20 +101,61 @@ func ResolveToken(tool string) (string, TokenSource, error) { // A corrupt shared CONFIG file only blocks the migration source; // it must not kill the command. Defer migration, warn once, and // still resolve the token from the keyring (non-migrating). - // Genuine keyring-backend errors still propagate. + // Genuine keyring-backend errors still propagate (wrapped). if errors.Is(err, credstore.ErrCorruptStore) { warnCorruptOnce(err) ns, nerr := OpenNoMigrate() if nerr != nil { - return "", SourceNone, nerr + return "", SourceNone, keyringUnavailableError(tool, nerr) } defer func() { _ = ns.Close() }() - return resolveFromStore(ns) + tok, src, rerr := resolveFromStore(ns) + if rerr != nil { + return "", SourceNone, keyringUnavailableError(tool, rerr) + } + return tok, src, nil } - return "", SourceNone, err + return "", SourceNone, keyringUnavailableError(tool, err) } defer func() { _ = s.Close() }() - return resolveFromStore(s) + tok, src, rerr := resolveFromStore(s) + if rerr != nil { + return "", SourceNone, keyringUnavailableError(tool, rerr) + } + return tok, src, nil +} + +// keyringUnavailableError wraps a genuine keyring open/read failure with +// the documented escape hatches so the headless / no-keyring case +// (issue #384) is self-service instead of an opaque backend error. It is +// applied ONLY to real backend errors — the §1.8 corrupt-shared-config +// graceful path never reaches it, and a benign "no token present" result +// is a nil error that is never wrapped. +// +// The wrapped error keeps the original via %w, so callers and tests that +// classify with errors.Is (e.g. credstore.ErrSecretServiceFailClosed) +// still match. The message names, in resolution order, the runtime escape +// hatches the Secret-Handling Standard already supports: +// - the per-tool / shared API-token env vars (resolved before the +// keyring is ever opened — the canonical headless answer); +// - the encrypted-file backend (--backend file) and its passphrase env +// var, plus the pass backend (--backend pass) for password-store users. +// +// No new plaintext path is introduced: the env var and file backend are +// the standard's own §1.4 fallbacks, surfaced here so users discover them +// instead of hitting a silent keyring-unlock prompt that never appears. +func keyringUnavailableError(tool string, err error) error { + if err == nil { + return nil + } + envVars := strings.Join(envVarsFor(tool), " or ") + return fmt.Errorf( + "could not read the API token from the OS keyring (%w). "+ + "If you are running headless or have no usable keyring, supply the token via %s, "+ + "or select the encrypted-file backend with --backend file "+ + "(passphrase via %s) or the pass backend with --backend pass. "+ + "See `cfl config show` / `jtk config show` for the active backend", + err, envVars, passphraseEnvVar(Service)) } // ResolveTokenNoMigrate is the DIAGNOSTIC resolver (`config show` source diff --git a/shared/keyring/resolve_test.go b/shared/keyring/resolve_test.go new file mode 100644 index 00000000..3cbbb0dd --- /dev/null +++ b/shared/keyring/resolve_test.go @@ -0,0 +1,144 @@ +package keyring + +import ( + "errors" + "os" + "strings" + "testing" + + cccredstore "github.com/open-cli-collective/cli-common/credstore" +) + +// keyringUnavailableError must wrap the underlying backend error (so +// errors.Is still classifies it) and name every documented runtime +// escape hatch so the headless / no-keyring case (issue #384) is +// self-service. It must be a no-op on a nil error. +func TestKeyringUnavailableError_WrapsAndNamesEscapeHatches(t *testing.T) { + t.Parallel() + + if got := keyringUnavailableError(ToolCFL, nil); got != nil { + t.Fatalf("nil error must not be wrapped; got %v", got) + } + + base := errors.New("dbus: keyring is locked") + for _, tc := range []struct { + tool string + wantVars []string + }{ + {ToolCFL, []string{"CFL_API_TOKEN", "ATLASSIAN_API_TOKEN"}}, + {ToolJTK, []string{"JIRA_API_TOKEN", "ATLASSIAN_API_TOKEN"}}, + } { + err := keyringUnavailableError(tc.tool, base) + if err == nil { + t.Fatalf("%s: expected a wrapped error", tc.tool) + } + // Underlying error preserved for errors.Is classification. + if !errors.Is(err, base) { + t.Fatalf("%s: wrapped error must keep the underlying error; got %v", tc.tool, err) + } + msg := err.Error() + // Names the per-tool and shared env vars (the headless answer: + // resolved before the keyring is ever opened). + for _, v := range tc.wantVars { + if !strings.Contains(msg, v) { + t.Fatalf("%s: error must name env var %s; got %v", tc.tool, v, err) + } + } + // Names the file + pass backend escape hatches and the passphrase + // env var (the standard's §1.4 fallbacks). + for _, want := range []string{"--backend file", "--backend pass", passphraseEnvVar(Service)} { + if !strings.Contains(msg, want) { + t.Fatalf("%s: error must name %q; got %v", tc.tool, want, err) + } + } + // Never the secret material itself (there is none here, but guard + // the message shape against a future refactor that interpolates a + // value). + if strings.Contains(msg, "dbus: keyring is locked") { + // The underlying message is allowed (it carries no secret); this + // assertion just documents that we wrap, not replace, it. + if !strings.Contains(msg, "could not read the API token") { + t.Fatalf("%s: wrapper prose missing; got %v", tc.tool, err) + } + } + } +} + +// ResolveToken must surface the actionable escape-hatch message — not an +// opaque backend error — when the keyring backend cannot be opened. We +// force a genuine open failure by selecting an unknown backend, which +// cli-common rejects with ErrBackendNotImplemented (OS-independent). +func TestResolveToken_KeyringOpenFailure_WrappedActionable(t *testing.T) { + hermetic(t) + // Override the file backend the hermetic harness selects with a bogus + // one so credstore.Open fails closed before any read/migration. + t.Setenv(cccredstore.BackendEnvVar(Service), "no-such-backend") + + _, _, err := ResolveToken(ToolJTK) + if err == nil { + t.Fatal("expected a keyring-open error") + } + // Underlying classification still works. + if !errors.Is(err, cccredstore.ErrBackendNotImplemented) { + t.Fatalf("must preserve the underlying backend error for errors.Is; got %v", err) + } + msg := err.Error() + for _, want := range []string{ + "JIRA_API_TOKEN", "ATLASSIAN_API_TOKEN", + "--backend file", "--backend pass", + passphraseEnvVar(Service), + } { + if !strings.Contains(msg, want) { + t.Fatalf("error must name %q so the failure is self-service; got %v", want, err) + } + } +} + +// The escape hatch the wrapped message advertises must actually work: with +// the API-token env var set, ResolveToken returns it and NEVER opens the +// keyring — so a broken/locked backend (or no keyring at all, the issue +// #384 headless case) is fully bypassed. This is the config-file-present +// user's fix: export the token, don't touch the keystore. +func TestResolveToken_EnvBypassesBrokenKeyring(t *testing.T) { + hermetic(t) + // A backend that would fail if opened — proving env short-circuits + // before any keyring access. + t.Setenv(cccredstore.BackendEnvVar(Service), "no-such-backend") + const envTok = "ENV-TOKEN-pqrSTU" //nolint:gosec // G101: test fixture, not a real credential + t.Setenv("ATLASSIAN_API_TOKEN", envTok) + + tok, src, err := ResolveToken(ToolCFL) + if err != nil { + t.Fatalf("env token must bypass the keyring entirely; got error %v", err) + } + if tok != envTok { + t.Fatalf("token = %q, want %q", tok, envTok) + } + if src != SourceEnv { + t.Fatalf("source = %q, want %q", src, SourceEnv) + } +} + +// Regression guard for the graceful-degradation contract: a corrupt +// shared config must still resolve the token from a WORKING keyring with +// NO error — the escape-hatch wrapper must not leak into that path (it is +// reserved for genuine backend failures). +func TestResolveToken_CorruptSharedConfig_NotWrappedWhenKeyringWorks(t *testing.T) { + hermetic(t) + if err := SetCredential(strings.NewReader(secret), ""); err != nil { + t.Fatalf("seed: %v", err) + } + sharedPath := sharedConfigPath(t) + // Neither valid YAML nor JSON → credstore.Load wraps ErrCorruptStore. + if err := os.WriteFile(sharedPath, []byte(":::not yaml: ["), 0o600); err != nil { + t.Fatal(err) + } + + tok, src, err := ResolveToken(ToolCFL) + if err != nil { + t.Fatalf("corrupt shared config with a working keyring must NOT error; got %v", err) + } + if tok != secret || src != SourceKeyAPI { + t.Fatalf("token=%q src=%q, want %q / %q", tok, src, secret, SourceKeyAPI) + } +} From 036c91cd1bfd5482e18b937cc370c92d793ded2d Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 17 Jun 2026 09:46:25 -0400 Subject: [PATCH 2/2] fix(keyring): wrap only backend-unavailability errors; parameterize backend hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the keyring escape-hatch error: - keyringUnavailableError applied the headless/no-keyring advice ("no usable keyring", env vars, --backend file/pass) to EVERY non-nil error from resolveFromStore. A decode failure, format mismatch, or store sentinel (e.g. ErrStoreClosed) would be mislabeled with that advice. Introduce an isBackendError predicate matching the backend-availability sentinels (ErrSecretServiceFailClosed, ErrBackendNotImplemented, ErrFilePassphraseRequired) and wrap only when it matches; all other errors propagate unwrapped (errors.Is still classifies them). The function's "ONLY real backend errors" contract is now enforced, not just documented, and applies uniformly to every call site including the ErrCorruptStore branch. - The "active backend" hint hardcoded both tool names (`cfl config show` / `jtk config show`) despite taking a tool param; a third tool would get the wrong hint. Derive it from tool: `%s config show`. - Cover the previously-untested branch where Open succeeds but resolveFromStore then fails: resolveFromStore is now a package var test seam so a fake can return a backend (wrapped → actionable) or a non-backend (unwrapped) read error, asserting the predicate end to end. --- shared/keyring/resolve.go | 57 +++++++++++--- shared/keyring/resolve_test.go | 131 +++++++++++++++++++++++++++++---- 2 files changed, 165 insertions(+), 23 deletions(-) diff --git a/shared/keyring/resolve.go b/shared/keyring/resolve.go index 5279fc31..720fe685 100644 --- a/shared/keyring/resolve.go +++ b/shared/keyring/resolve.go @@ -7,6 +7,8 @@ import ( "strings" "sync" + cccredstore "github.com/open-cli-collective/cli-common/credstore" + "github.com/open-cli-collective/atlassian-go/credstore" ) @@ -101,7 +103,8 @@ func ResolveToken(tool string) (string, TokenSource, error) { // A corrupt shared CONFIG file only blocks the migration source; // it must not kill the command. Defer migration, warn once, and // still resolve the token from the keyring (non-migrating). - // Genuine keyring-backend errors still propagate (wrapped). + // Genuine keyring-backend errors still propagate (wrapped with + // the escape hatches); other failures propagate unwrapped. if errors.Is(err, credstore.ErrCorruptStore) { warnCorruptOnce(err) ns, nerr := OpenNoMigrate() @@ -125,12 +128,35 @@ func ResolveToken(tool string) (string, TokenSource, error) { return tok, src, nil } -// keyringUnavailableError wraps a genuine keyring open/read failure with -// the documented escape hatches so the headless / no-keyring case -// (issue #384) is self-service instead of an opaque backend error. It is -// applied ONLY to real backend errors — the §1.8 corrupt-shared-config -// graceful path never reaches it, and a benign "no token present" result -// is a nil error that is never wrapped. +// isBackendError reports whether err is a genuine keyring-BACKEND +// availability failure — the only class for which the headless / +// no-keyring escape-hatch advice (env vars, --backend file/pass) is +// actually accurate. It matches the cli-common credstore sentinels that +// mean "the backend itself could not be opened or used": +// - ErrSecretServiceFailClosed: Secret Service present but locked/denied +// (the issue #384 headless case); +// - ErrBackendNotImplemented: the selected/default backend is unavailable +// on this platform/build; +// - ErrFilePassphraseRequired: the file backend cannot open without its +// passphrase (the advice names that env var). +// +// A credential-decode failure, a format mismatch, ErrStoreClosed, or any +// other non-backend error returns false so it propagates UNWRAPPED rather +// than being mislabeled with "no usable keyring" remedies. +func isBackendError(err error) bool { + return errors.Is(err, cccredstore.ErrSecretServiceFailClosed) || + errors.Is(err, cccredstore.ErrBackendNotImplemented) || + errors.Is(err, cccredstore.ErrFilePassphraseRequired) +} + +// keyringUnavailableError wraps a genuine keyring backend-availability +// failure with the documented escape hatches so the headless / no-keyring +// case (issue #384) is self-service instead of an opaque backend error. +// It wraps ONLY real backend errors (see isBackendError): the §1.8 +// corrupt-shared-config graceful path never reaches it, a benign "no token +// present" result is a nil error, and a non-backend failure (decode, +// format mismatch, store sentinel) is returned UNWRAPPED so it is not +// mislabeled with "no usable keyring" advice that does not apply. // // The wrapped error keeps the original via %w, so callers and tests that // classify with errors.Is (e.g. credstore.ErrSecretServiceFailClosed) @@ -148,14 +174,20 @@ func keyringUnavailableError(tool string, err error) error { if err == nil { return nil } + // Only backend-availability failures get the headless/no-keyring + // remedies; anything else propagates unwrapped (the advice would be + // misleading for, e.g., a decode failure or ErrStoreClosed). + if !isBackendError(err) { + return err + } envVars := strings.Join(envVarsFor(tool), " or ") return fmt.Errorf( "could not read the API token from the OS keyring (%w). "+ "If you are running headless or have no usable keyring, supply the token via %s, "+ "or select the encrypted-file backend with --backend file "+ "(passphrase via %s) or the pass backend with --backend pass. "+ - "See `cfl config show` / `jtk config show` for the active backend", - err, envVars, passphraseEnvVar(Service)) + "See `%s config show` for the active backend", + err, envVars, passphraseEnvVar(Service), tool) } // ResolveTokenNoMigrate is the DIAGNOSTIC resolver (`config show` source @@ -175,7 +207,12 @@ func ResolveTokenNoMigrate(tool string) (string, TokenSource, error) { // resolveFromStore reads the single shared api_token. One key per logical // credential (§1.11.10): jtk and cfl resolve the same key. -func resolveFromStore(s *Store) (string, TokenSource, error) { +// +// It is a package var (not a plain func) purely as a test seam: a test +// must be able to exercise the "Open succeeded but the read failed" branch +// of ResolveToken without a backend that can fail mid-read on every OS. +// Production code never reassigns it. +var resolveFromStore = func(s *Store) (string, TokenSource, error) { if v, ok, err := s.get(KeyAPIToken); err != nil { return "", SourceNone, err } else if ok { diff --git a/shared/keyring/resolve_test.go b/shared/keyring/resolve_test.go index 3cbbb0dd..9f0fe924 100644 --- a/shared/keyring/resolve_test.go +++ b/shared/keyring/resolve_test.go @@ -2,6 +2,7 @@ package keyring import ( "errors" + "fmt" "os" "strings" "testing" @@ -9,10 +10,11 @@ import ( cccredstore "github.com/open-cli-collective/cli-common/credstore" ) -// keyringUnavailableError must wrap the underlying backend error (so -// errors.Is still classifies it) and name every documented runtime +// keyringUnavailableError must wrap a genuine backend-availability error +// (so errors.Is still classifies it) and name every documented runtime // escape hatch so the headless / no-keyring case (issue #384) is -// self-service. It must be a no-op on a nil error. +// self-service. The backend hint is parameterized off the tool. It must +// be a no-op on a nil error. func TestKeyringUnavailableError_WrapsAndNamesEscapeHatches(t *testing.T) { t.Parallel() @@ -20,7 +22,10 @@ func TestKeyringUnavailableError_WrapsAndNamesEscapeHatches(t *testing.T) { t.Fatalf("nil error must not be wrapped; got %v", got) } - base := errors.New("dbus: keyring is locked") + // A genuine backend-availability error (the only class that gets the + // headless escape-hatch advice). It carries an underlying message we + // can assert is preserved, not replaced. + base := fmt.Errorf("secret-service is locked: %w", cccredstore.ErrSecretServiceFailClosed) for _, tc := range []struct { tool string wantVars []string @@ -51,15 +56,42 @@ func TestKeyringUnavailableError_WrapsAndNamesEscapeHatches(t *testing.T) { t.Fatalf("%s: error must name %q; got %v", tc.tool, want, err) } } - // Never the secret material itself (there is none here, but guard - // the message shape against a future refactor that interpolates a - // value). - if strings.Contains(msg, "dbus: keyring is locked") { - // The underlying message is allowed (it carries no secret); this - // assertion just documents that we wrap, not replace, it. - if !strings.Contains(msg, "could not read the API token") { - t.Fatalf("%s: wrapper prose missing; got %v", tc.tool, err) - } + // The "active backend" hint is parameterized off the tool, not + // hardcoded — a third tool would otherwise get a wrong `cfl`/`jtk` + // suggestion (review comment 2). + if !strings.Contains(msg, "`"+tc.tool+" config show`") { + t.Fatalf("%s: backend hint must be derived from the tool; got %v", tc.tool, err) + } + // Wrapper prose present (we wrap, not replace, the backend message). + if !strings.Contains(msg, "could not read the API token") { + t.Fatalf("%s: wrapper prose missing; got %v", tc.tool, err) + } + } +} + +// keyringUnavailableError must NOT apply the headless / no-keyring escape +// hatches to a NON-backend error (a decode failure, format mismatch, +// ErrStoreClosed, etc.): that advice would be misleading. Such errors are +// returned UNWRAPPED (review comment 1). +func TestKeyringUnavailableError_NonBackendErrorNotWrapped(t *testing.T) { + t.Parallel() + + for _, base := range []error{ + errors.New("decode api_token: invalid base64 payload"), + fmt.Errorf("read api_token: %w", cccredstore.ErrStoreClosed), + } { + got := keyringUnavailableError(ToolCFL, base) + // Identity is preserved (returned verbatim, not re-wrapped): the + // message is byte-for-byte the original and errors.Is still matches. + if got.Error() != base.Error() { + t.Fatalf("non-backend error must be returned unwrapped; got %q (want %q)", got, base) + } + if !errors.Is(got, base) { + t.Fatalf("non-backend error must still classify via errors.Is; got %v", got) + } + if strings.Contains(got.Error(), "--backend file") || + strings.Contains(got.Error(), "no usable keyring") { + t.Fatalf("non-backend error must not gain keyring escape-hatch advice; got %v", got) } } } @@ -119,6 +151,79 @@ func TestResolveToken_EnvBypassesBrokenKeyring(t *testing.T) { } } +// The companion to the open-failure case: when Open SUCCEEDS but the +// subsequent read fails with a genuine backend error (e.g. the secret +// service goes locked/denied between open and read), ResolveToken must +// still surface the actionable escape-hatch message — not the opaque +// backend error — and still classify via errors.Is. We inject a +// resolveFromStore that returns ErrSecretServiceFailClosed because no +// backend can be made to open cleanly yet fail mid-read on every OS. +func TestResolveToken_StoreReadFailure_WrappedActionable(t *testing.T) { + hermetic(t) + // A real, working backend so Open() succeeds and we exercise the + // post-open read branch (not the open-failure branch). + if err := SetCredential(strings.NewReader(secret), ""); err != nil { + t.Fatalf("seed: %v", err) + } + orig := resolveFromStore + t.Cleanup(func() { resolveFromStore = orig }) + resolveFromStore = func(*Store) (string, TokenSource, error) { + // A backend-availability failure (the locked/denied secret-service + // case) — exactly the class isBackendError matches, so it is wrapped. + return "", SourceNone, fmt.Errorf("read api_token: %w", cccredstore.ErrSecretServiceFailClosed) + } + + _, _, err := ResolveToken(ToolJTK) + if err == nil { + t.Fatal("expected a keyring-read error") + } + // Underlying classification still works through the wrapper. + if !errors.Is(err, cccredstore.ErrSecretServiceFailClosed) { + t.Fatalf("must preserve the underlying backend error for errors.Is; got %v", err) + } + msg := err.Error() + for _, want := range []string{ + "JIRA_API_TOKEN", "ATLASSIAN_API_TOKEN", + "--backend file", "--backend pass", + passphraseEnvVar(Service), + } { + if !strings.Contains(msg, want) { + t.Fatalf("error must name %q so the failure is self-service; got %v", want, err) + } + } +} + +// The flip side of the predicate (comment 1): a NON-backend read failure +// (e.g. a credential-decode / format error) must propagate UNWRAPPED — it +// must NOT be mislabeled with the headless / no-keyring escape-hatch +// advice, which would be misleading. The underlying error is preserved +// verbatim for errors.Is. +func TestResolveToken_NonBackendReadFailure_NotWrapped(t *testing.T) { + hermetic(t) + if err := SetCredential(strings.NewReader(secret), ""); err != nil { + t.Fatalf("seed: %v", err) + } + orig := resolveFromStore + t.Cleanup(func() { resolveFromStore = orig }) + decodeErr := errors.New("decode api_token: invalid base64 payload") + resolveFromStore = func(*Store) (string, TokenSource, error) { + return "", SourceNone, decodeErr + } + + _, _, err := ResolveToken(ToolCFL) + if err == nil { + t.Fatal("expected a read error") + } + if !errors.Is(err, decodeErr) { + t.Fatalf("non-backend error must propagate; got %v", err) + } + // The escape-hatch prose must NOT appear: this is not a no-keyring case. + if strings.Contains(err.Error(), "no usable keyring") || + strings.Contains(err.Error(), "--backend file") { + t.Fatalf("non-backend read failure must not get keyring escape-hatch advice; got %v", err) + } +} + // Regression guard for the graceful-degradation contract: a corrupt // shared config must still resolve the token from a WORKING keyring with // NO error — the escape-hatch wrapper must not leak into that path (it is