fix(credentials): raise a concrete error when the keyring backend fails
The resolver caught bare `Exception` around keyring reads and writes. That could not be narrowed at the call site: the errors originate in python-keyring, an optional dependency, so `keyring.errors.KeyringError` is not referenceable without importing the package unconditionally. Move the open-ended catch to the boundary where keyring is already imported. KeyringTokenProvider.get/set now wrap the backend calls and re-raise as KeyringOperationError, preserving the original exception as __cause__. The resolver catches that concrete type instead. KeyringOperationError is deliberately distinct from KeyringUnavailableError: "no usable backend" and "backend present but the call failed" are different states, and the resolver treats them differently — the former warns once and visibly, the latter degrades silently. Both degradation paths are unchanged: a failed write still lets the token be used for the current run, a failed read still falls through to the prompt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,7 @@
|
|||||||
from .resolver import ResolutionOptions, TokenResolver
|
from .resolver import ResolutionOptions, TokenResolver
|
||||||
from .types import (
|
from .types import (
|
||||||
CredentialError,
|
CredentialError,
|
||||||
|
KeyringOperationError,
|
||||||
KeyringUnavailableError,
|
KeyringUnavailableError,
|
||||||
NoCredentialsError,
|
NoCredentialsError,
|
||||||
TokenRequest,
|
TokenRequest,
|
||||||
@@ -11,6 +12,7 @@ from .types import (
|
|||||||
|
|
||||||
__all__ = [
|
__all__ = [
|
||||||
"CredentialError",
|
"CredentialError",
|
||||||
|
"KeyringOperationError",
|
||||||
"KeyringUnavailableError",
|
"KeyringUnavailableError",
|
||||||
"NoCredentialsError",
|
"NoCredentialsError",
|
||||||
"ResolutionOptions",
|
"ResolutionOptions",
|
||||||
|
|||||||
@@ -2,10 +2,14 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import Optional
|
|
||||||
|
|
||||||
from ..store_keys import build_keyring_key
|
from ..store_keys import build_keyring_key
|
||||||
from ..types import KeyringUnavailableError, TokenRequest, TokenResult
|
from ..types import (
|
||||||
|
KeyringOperationError,
|
||||||
|
KeyringUnavailableError,
|
||||||
|
TokenRequest,
|
||||||
|
TokenResult,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _import_keyring():
|
def _import_keyring():
|
||||||
@@ -41,10 +45,15 @@ class KeyringTokenProvider:
|
|||||||
|
|
||||||
source_name: str = "keyring"
|
source_name: str = "keyring"
|
||||||
|
|
||||||
def get(self, request: TokenRequest) -> Optional[TokenResult]:
|
def get(self, request: TokenRequest) -> TokenResult | None:
|
||||||
keyring = _import_keyring()
|
keyring = _import_keyring()
|
||||||
key = build_keyring_key(request.provider_kind, request.host, request.owner)
|
key = build_keyring_key(request.provider_kind, request.host, request.owner)
|
||||||
token = keyring.get_password(key.service, key.username)
|
try:
|
||||||
|
token = keyring.get_password(key.service, key.username)
|
||||||
|
except Exception as exc:
|
||||||
|
raise KeyringOperationError(
|
||||||
|
f"Reading the keyring entry for {key.service!r} failed."
|
||||||
|
) from exc
|
||||||
if token:
|
if token:
|
||||||
return TokenResult(token=token.strip(), source=self.source_name)
|
return TokenResult(token=token.strip(), source=self.source_name)
|
||||||
return None
|
return None
|
||||||
@@ -52,4 +61,9 @@ class KeyringTokenProvider:
|
|||||||
def set(self, request: TokenRequest, token: str) -> None:
|
def set(self, request: TokenRequest, token: str) -> None:
|
||||||
keyring = _import_keyring()
|
keyring = _import_keyring()
|
||||||
key = build_keyring_key(request.provider_kind, request.host, request.owner)
|
key = build_keyring_key(request.provider_kind, request.host, request.owner)
|
||||||
keyring.set_password(key.service, key.username, token)
|
try:
|
||||||
|
keyring.set_password(key.service, key.username, token)
|
||||||
|
except Exception as exc:
|
||||||
|
raise KeyringOperationError(
|
||||||
|
f"Writing the keyring entry for {key.service!r} failed."
|
||||||
|
) from exc
|
||||||
|
|||||||
@@ -3,13 +3,13 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import sys
|
import sys
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import Optional
|
|
||||||
|
|
||||||
from .providers.env import EnvTokenProvider
|
from .providers.env import EnvTokenProvider
|
||||||
from .providers.gh import GhTokenProvider
|
from .providers.gh import GhTokenProvider
|
||||||
from .providers.keyring import KeyringTokenProvider
|
from .providers.keyring import KeyringTokenProvider
|
||||||
from .providers.prompt import PromptTokenProvider
|
from .providers.prompt import PromptTokenProvider
|
||||||
from .types import (
|
from .types import (
|
||||||
|
KeyringOperationError,
|
||||||
KeyringUnavailableError,
|
KeyringUnavailableError,
|
||||||
NoCredentialsError,
|
NoCredentialsError,
|
||||||
TokenRequest,
|
TokenRequest,
|
||||||
@@ -76,7 +76,7 @@ class TokenResolver:
|
|||||||
self,
|
self,
|
||||||
request: TokenRequest,
|
request: TokenRequest,
|
||||||
opts: ResolutionOptions,
|
opts: ResolutionOptions,
|
||||||
) -> Optional[TokenResult]:
|
) -> TokenResult | None:
|
||||||
"""
|
"""
|
||||||
Prompt for a token and optionally store it in keyring.
|
Prompt for a token and optionally store it in keyring.
|
||||||
If keyring is unavailable, still return the token for this run.
|
If keyring is unavailable, still return the token for this run.
|
||||||
@@ -93,8 +93,8 @@ class TokenResolver:
|
|||||||
self._keyring.set(request, prompt_res.token) # overwrite is fine
|
self._keyring.set(request, prompt_res.token) # overwrite is fine
|
||||||
except KeyringUnavailableError as exc:
|
except KeyringUnavailableError as exc:
|
||||||
self._warn_keyring_unavailable(exc)
|
self._warn_keyring_unavailable(exc)
|
||||||
except Exception:
|
except KeyringOperationError:
|
||||||
# If keyring cannot store, still use token for this run.
|
# Storing failed; the token is still valid for this run.
|
||||||
pass
|
pass
|
||||||
|
|
||||||
return prompt_res
|
return prompt_res
|
||||||
@@ -103,8 +103,8 @@ class TokenResolver:
|
|||||||
self,
|
self,
|
||||||
provider_kind: str,
|
provider_kind: str,
|
||||||
host: str,
|
host: str,
|
||||||
owner: Optional[str] = None,
|
owner: str | None = None,
|
||||||
options: Optional[ResolutionOptions] = None,
|
options: ResolutionOptions | None = None,
|
||||||
) -> TokenResult:
|
) -> TokenResult:
|
||||||
opts = options or ResolutionOptions()
|
opts = options or ResolutionOptions()
|
||||||
request = TokenRequest(provider_kind=provider_kind, host=host, owner=owner)
|
request = TokenRequest(provider_kind=provider_kind, host=host, owner=owner)
|
||||||
@@ -135,8 +135,8 @@ class TokenResolver:
|
|||||||
except KeyringUnavailableError as exc:
|
except KeyringUnavailableError as exc:
|
||||||
# Show a helpful warning once, then continue (prompt fallback).
|
# Show a helpful warning once, then continue (prompt fallback).
|
||||||
self._warn_keyring_unavailable(exc)
|
self._warn_keyring_unavailable(exc)
|
||||||
except Exception:
|
except KeyringOperationError:
|
||||||
# Unknown keyring errors: do not block prompting; still avoid hard crash.
|
# Reading failed; fall through to the prompt.
|
||||||
pass
|
pass
|
||||||
|
|
||||||
# 3) Prompt (optional)
|
# 3) Prompt (optional)
|
||||||
|
|||||||
@@ -2,7 +2,6 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import Optional
|
|
||||||
|
|
||||||
|
|
||||||
class CredentialError(RuntimeError):
|
class CredentialError(RuntimeError):
|
||||||
@@ -17,13 +16,17 @@ class KeyringUnavailableError(CredentialError):
|
|||||||
"""Raised when keyring is requested but no backend is available."""
|
"""Raised when keyring is requested but no backend is available."""
|
||||||
|
|
||||||
|
|
||||||
|
class KeyringOperationError(CredentialError):
|
||||||
|
"""Raised when a keyring read or write fails inside the backend."""
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class TokenRequest:
|
class TokenRequest:
|
||||||
"""Parameters describing which token we need."""
|
"""Parameters describing which token we need."""
|
||||||
|
|
||||||
provider_kind: str # e.g. "gitea", "github"
|
provider_kind: str # e.g. "gitea", "github"
|
||||||
host: str # e.g. "git.example.org" or "github.com"
|
host: str # e.g. "git.example.org" or "github.com"
|
||||||
owner: Optional[str] = None # optional org/user
|
owner: str | None = None # optional org/user
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
|
|||||||
Reference in New Issue
Block a user