Skip to content

fix(auth): distinguish missing credentials from operational keyring failures (#1) [0xcFDa9f32d292661740a6d0B4c00867E34c05c56D] - #7

Open
meridianmindx wants to merge 1 commit into
condoraltidoi32:mainfrom
meridianmindx:fix/keyring-error-handling
Open

fix(auth): distinguish missing credentials from operational keyring failures (#1) [0xcFDa9f32d292661740a6d0B4c00867E34c05c56D]#7
meridianmindx wants to merge 1 commit into
condoraltidoi32:mainfrom
meridianmindx:fix/keyring-error-handling

Conversation

@meridianmindx

Copy link
Copy Markdown

Summary

Fixes #1 — Differentiates missing credentials from operational keyring failures and prevents silent unauthenticated API requests in gh api.

Root Cause

Previously, operational errors from secret storage (such as locked keychain, disconnected D-Bus daemon, access denied) were treated equivalently to missing credentials (ErrNotFound). Consequently, gh api would swallow the error, proceed with an unauthenticated request, and cause confusing downstream HTTP 404 or 403 rate-limit errors.

Changes & Architecture

  1. pkg/authtoken:
    • Explicitly defines ErrNotFound for absent credentials.
    • Wraps operational errors in KeyringOpError containing target host and underlying root cause.
    • TokenFromKeyring only returns ErrNotFound when a secret is absent, while strictly propagating operational failures.
  2. internal/config:
    • AuthTokenForHost evaluates resolution hierarchy: GH_TOKEN / GITHUB_TOKEN -> System Keyring -> hosts.yml.
    • Halts resolution chain immediately upon encountering operational keyring errors (IsKeyringOpError), preventing silent fallback.
    • Fallback to unauthenticated execution only proceeds when credentials are cleanly not found across all providers.
  3. pkg/cmd/factory & pkg/cmd/api:
    • HttpClientFunc surfaces operational failures.
    • RunApi catches keyring errors, halts execution immediately with non-zero exit code, and writes actionable diagnostics to stderr (explaining root cause and suggesting GH_TOKEN bypass).
  4. Comprehensive Test Suite:
    • pkg/authtoken: missing token vs operational error tests.
    • internal/config: resolution precedence, clean fallback, and error propagation tests.
    • pkg/cmd/factory: factory HTTP client construction error handling.
    • pkg/cmd/api: command execution, stderr output validation, unauthenticated public requests, and authenticated header injection.

Verification

$ go test -v ./...
=== RUN   TestMainScaffold
--- PASS: TestMainScaffold (0.00s)
=== RUN   TestAuthTokenForHost
--- PASS: TestAuthTokenForHost (0.00s)
=== RUN   TestTokenFromKeyring
--- PASS: TestTokenFromKeyring (0.00s)
=== RUN   TestRunApi
--- PASS: TestRunApi (0.00s)
=== RUN   TestFactoryHttpClient
--- PASS: TestFactoryHttpClient (0.00s)
PASS

Payout Address

  • Base EVM: 0xcFDa9f32d292661740a6d0B4c00867E34c05c56D

Fixes #1

…ailures (condoraltidoi32#1)

- Distinguish ErrNotFound from operational keyring daemon/system failures in authtoken package
- Propagate operational errors up through config.AuthTokenForHost and factory.HttpClientFunc without swallowing or silent fallback
- Surface clear, actionable troubleshooting diagnostics to stderr in RunApi when keyring fails
- Comprehensive unit tests for authtoken, config resolution chain, factory, and API command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🎯 Fail with descriptive error instead of silently sending unauthenticated API requests when keyring access fails

1 participant