From 6dfceff05ace4159c10e977b586cd977a19fdb0e Mon Sep 17 00:00:00 2001 From: akazlow Date: Fri, 12 Jun 2026 15:58:36 -0400 Subject: [PATCH 1/6] fix(mural): fallback to file backend when keyring is empty --- .../mural/scripts/mural/_backends.py | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/.github/skills/experimental/mural/scripts/mural/_backends.py b/.github/skills/experimental/mural/scripts/mural/_backends.py index 29f060995..83d4664a7 100644 --- a/.github/skills/experimental/mural/scripts/mural/_backends.py +++ b/.github/skills/experimental/mural/scripts/mural/_backends.py @@ -33,8 +33,8 @@ _NullBackend, _state, ) -from ._constants import _LINE_RE, ENV_NONINTERACTIVE -from ._credentials import _resolve_credential_file +from ._constants import _KNOWN_CREDENTIAL_KEYS, _LINE_RE, ENV_NONINTERACTIVE +from ._credentials import _resolve_credential_file, _service_name_for from ._exceptions import MuralError from ._protocols import CredentialBackend @@ -230,6 +230,28 @@ def resolve_backend(profile: str = "default") -> CredentialBackend: elif selector == "auto": try: selected = KeyringBackend() + service = _service_name_for(profile) + keyring_populated = False + for key in _KNOWN_CREDENTIAL_KEYS: + value = selected.get(service, key) + if value: + keyring_populated = True + break + if not keyring_populated: + file_backend = FileBackend(file_path) + if file_path.exists(): + _check_credential_file_perms(file_path, os.environ) + file_entries = file_backend._read_all() + file_populated = any(file_entries.get(k) for k in _KNOWN_CREDENTIAL_KEYS) + if file_populated: + if profile not in _state.seen_fallback_warn(): + _state.seen_fallback_warn().add(profile) + _emit( + f"keyring backend available but empty for profile {profile!r}; " + f"using file backend at {file_path}", + level=logging.WARNING, + ) + selected = file_backend except _KeyringUnavailable as exc: if profile not in _state.seen_fallback_warn(): _state.seen_fallback_warn().add(profile) From 389999580c3303fcae5e28a059afcb3d306e3acc Mon Sep 17 00:00:00 2001 From: akazlow Date: Mon, 22 Jun 2026 12:33:28 -0400 Subject: [PATCH 2/6] fix(mural): fix linting errors - break long lines for ruff compliance --- .../experimental/mural/scripts/mural/_backends.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/skills/experimental/mural/scripts/mural/_backends.py b/.github/skills/experimental/mural/scripts/mural/_backends.py index 83d4664a7..14438e59e 100644 --- a/.github/skills/experimental/mural/scripts/mural/_backends.py +++ b/.github/skills/experimental/mural/scripts/mural/_backends.py @@ -242,15 +242,18 @@ def resolve_backend(profile: str = "default") -> CredentialBackend: if file_path.exists(): _check_credential_file_perms(file_path, os.environ) file_entries = file_backend._read_all() - file_populated = any(file_entries.get(k) for k in _KNOWN_CREDENTIAL_KEYS) + file_populated = any( + file_entries.get(k) for k in _KNOWN_CREDENTIAL_KEYS + ) if file_populated: if profile not in _state.seen_fallback_warn(): _state.seen_fallback_warn().add(profile) - _emit( - f"keyring backend available but empty for profile {profile!r}; " - f"using file backend at {file_path}", - level=logging.WARNING, + msg = ( + f"keyring backend available but empty for " + f"profile {profile!r}; " + f"using file backend at {file_path}" ) + _emit(msg, level=logging.WARNING) selected = file_backend except _KeyringUnavailable as exc: if profile not in _state.seen_fallback_warn(): From 06d0c215fd29aca69276a7ba253b7d66dbab3e9a Mon Sep 17 00:00:00 2001 From: akazlow Date: Tue, 23 Jun 2026 16:08:15 -0400 Subject: [PATCH 3/6] fix(mural): address PR review for backend fallback docs and tests --- .../mural/scripts/mural/_backends.py | 4 ++- .../mural/tests/test_credential_storage.py | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/.github/skills/experimental/mural/scripts/mural/_backends.py b/.github/skills/experimental/mural/scripts/mural/_backends.py index 14438e59e..f7e0020c9 100644 --- a/.github/skills/experimental/mural/scripts/mural/_backends.py +++ b/.github/skills/experimental/mural/scripts/mural/_backends.py @@ -212,7 +212,9 @@ def resolve_backend(profile: str = "default") -> CredentialBackend: ``MURAL_CREDENTIAL_BACKEND`` selects the backend (``auto`` default, ``keyring``, ``file``, ``env-only``). On ``auto``, KeyringBackend is tried first and falls back to FileBackend when ``_KeyringUnavailable`` - is raised; a one-shot WARN per profile records the fallback. After + is raised. Auto mode also falls back when keyring is available but has + no usable credentials and the file backend is populated. A one-shot + WARN per profile records whichever fallback path is taken. After backend selection (skipped for env-only), a probe checks whether the other persistent backend also holds non-empty values and emits a second one-shot WARN per ``(profile, selected_backend)`` pair when so. diff --git a/.github/skills/experimental/mural/tests/test_credential_storage.py b/.github/skills/experimental/mural/tests/test_credential_storage.py index e0f1bb37f..83c867ef5 100644 --- a/.github/skills/experimental/mural/tests/test_credential_storage.py +++ b/.github/skills/experimental/mural/tests/test_credential_storage.py @@ -377,6 +377,38 @@ def test_auto_returns_keyring_when_available( backend = mural_module.resolve_backend("default") assert isinstance(backend, mural_module.KeyringBackend) + def test_auto_falls_back_to_file_when_keyring_is_empty( + self, + mural_module: Any, + monkeypatch: pytest.MonkeyPatch, + tmp_path: pathlib.Path, + caplog: pytest.LogCaptureFixture, + ) -> None: + _isolate_credential_env(monkeypatch, tmp_path) + monkeypatch.setenv( + "MURAL_KEYRING_BACKEND", "keyrings.alt.file.PlaintextKeyring" + ) + + service = mural_module._service_name_for("default") + file_path = mural_module._resolve_credential_file("default", os.environ) + file_backend = mural_module.FileBackend(file_path) + file_backend.set(service, "MURAL_CLIENT_ID", "from-file") + + caplog.set_level(logging.WARNING, logger="mural") + for _ in range(3): + backend = mural_module.resolve_backend("default") + assert isinstance(backend, mural_module.FileBackend) + + warns = [ + r + for r in caplog.records + if r.levelno == logging.WARNING + and "keyring backend available but empty for profile 'default'" + in r.message + and "using file backend" in r.message + ] + assert len(warns) == 1 + def test_auto_falls_back_to_file_on_keyring_unavailable( self, mural_module: Any, From e6858311c5bbaa626ea74739a07ef82f0b2cdab9 Mon Sep 17 00:00:00 2001 From: akazlow Date: Tue, 23 Jun 2026 16:39:24 -0400 Subject: [PATCH 4/6] style(mural): apply ruff formatting in credential storage tests --- .../skills/experimental/mural/tests/test_credential_storage.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/skills/experimental/mural/tests/test_credential_storage.py b/.github/skills/experimental/mural/tests/test_credential_storage.py index 83c867ef5..2b9cf895e 100644 --- a/.github/skills/experimental/mural/tests/test_credential_storage.py +++ b/.github/skills/experimental/mural/tests/test_credential_storage.py @@ -403,8 +403,7 @@ def test_auto_falls_back_to_file_when_keyring_is_empty( r for r in caplog.records if r.levelno == logging.WARNING - and "keyring backend available but empty for profile 'default'" - in r.message + and "keyring backend available but empty for profile 'default'" in r.message and "using file backend" in r.message ] assert len(warns) == 1 From f7ff5cdd97574e6bc8f484aacaf67fe91fd5972c Mon Sep 17 00:00:00 2001 From: akazlow Date: Tue, 23 Jun 2026 16:52:26 -0400 Subject: [PATCH 5/6] refactor(mural): unify credential backend probes --- .../mural/scripts/mural/_backends.py | 31 +++++++------ .../mural/scripts/mural/_credentials.py | 44 ++++++++++++++----- .../mural/tests/test_credential_storage.py | 24 ++++++++++ 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/.github/skills/experimental/mural/scripts/mural/_backends.py b/.github/skills/experimental/mural/scripts/mural/_backends.py index f7e0020c9..1ac10a7e5 100644 --- a/.github/skills/experimental/mural/scripts/mural/_backends.py +++ b/.github/skills/experimental/mural/scripts/mural/_backends.py @@ -33,8 +33,12 @@ _NullBackend, _state, ) -from ._constants import _KNOWN_CREDENTIAL_KEYS, _LINE_RE, ENV_NONINTERACTIVE -from ._credentials import _resolve_credential_file, _service_name_for +from ._constants import _LINE_RE, ENV_NONINTERACTIVE +from ._credentials import ( + _backend_has_credentials, + _resolve_credential_file, + _service_name_for, +) from ._exceptions import MuralError from ._protocols import CredentialBackend @@ -233,19 +237,14 @@ def resolve_backend(profile: str = "default") -> CredentialBackend: try: selected = KeyringBackend() service = _service_name_for(profile) - keyring_populated = False - for key in _KNOWN_CREDENTIAL_KEYS: - value = selected.get(service, key) - if value: - keyring_populated = True - break + keyring_populated = _backend_has_credentials(selected, service) if not keyring_populated: file_backend = FileBackend(file_path) - if file_path.exists(): - _check_credential_file_perms(file_path, os.environ) - file_entries = file_backend._read_all() - file_populated = any( - file_entries.get(k) for k in _KNOWN_CREDENTIAL_KEYS + file_populated = _backend_has_credentials( + file_backend, + service, + enforce_file_perms=True, + file_env=os.environ, ) if file_populated: if profile not in _state.seen_fallback_warn(): @@ -266,6 +265,12 @@ def resolve_backend(profile: str = "default") -> CredentialBackend: level=logging.WARNING, ) selected = FileBackend(file_path) + _backend_has_credentials( + selected, + _service_name_for(profile), + enforce_file_perms=True, + file_env=os.environ, + ) else: raise MuralError( f"MURAL_CREDENTIAL_BACKEND={selector!r} is not one of " diff --git a/.github/skills/experimental/mural/scripts/mural/_credentials.py b/.github/skills/experimental/mural/scripts/mural/_credentials.py index 9d9dc63c1..6f3caf9f3 100644 --- a/.github/skills/experimental/mural/scripts/mural/_credentials.py +++ b/.github/skills/experimental/mural/scripts/mural/_credentials.py @@ -239,6 +239,35 @@ def _probe_keyring_availability() -> tuple[bool, str | None, str | None]: return _keyring_probe_cache +def _backend_has_credentials( + backend: CredentialBackend, + service: str, + *, + enforce_file_perms: bool = False, + file_env: Mapping[str, str] | None = None, +) -> bool: + """Return True when ``backend`` holds any non-empty credential value. + + FileBackend probes can optionally enforce the credential file permission + contract before reading so callers may choose between strict resolve-time + validation and best-effort warning-only inspection. + """ + if isinstance(backend, _pkg().FileBackend): + if not backend._path.exists(): + return False + if enforce_file_perms: + _pkg()._check_credential_file_perms( + backend._path, + file_env if file_env is not None else os.environ, + ) + entries = backend._read_all() + return any(entries.get(key) for key in _KNOWN_CREDENTIAL_KEYS) + for key in _KNOWN_CREDENTIAL_KEYS: + if backend.get(service, key): + return True + return False + + def _maybe_warn_concurrent_state( profile: str, selected: CredentialBackend, @@ -253,24 +282,19 @@ def _maybe_warn_concurrent_state( dedup_key = (profile, selected.name) if dedup_key in _pkg()._state.seen_concurrent_warn(): return - keyring_populated = False - file_populated = False service = _service_name_for(profile) try: probe_keyring = _pkg().KeyringBackend() - for key in _KNOWN_CREDENTIAL_KEYS: - value = probe_keyring.get(service, key) - if value: - keyring_populated = True - break + keyring_populated = _backend_has_credentials(probe_keyring, service) except _KeyringUnavailable: keyring_populated = False except Exception: # noqa: BLE001 - probe must never raise keyring_populated = False try: - if file_path.exists(): - entries = _pkg().FileBackend(file_path)._read_all() - file_populated = any(entries.get(k) for k in _KNOWN_CREDENTIAL_KEYS) + file_populated = _backend_has_credentials( + _pkg().FileBackend(file_path), + service, + ) except Exception: # noqa: BLE001 - probe must never raise file_populated = False if keyring_populated and file_populated: diff --git a/.github/skills/experimental/mural/tests/test_credential_storage.py b/.github/skills/experimental/mural/tests/test_credential_storage.py index 2b9cf895e..f1c017879 100644 --- a/.github/skills/experimental/mural/tests/test_credential_storage.py +++ b/.github/skills/experimental/mural/tests/test_credential_storage.py @@ -433,6 +433,30 @@ def _raise_unavailable(self: Any) -> None: ] assert len(warns) == 1 + @pytest.mark.skipif( + os.name == "nt", reason="POSIX-only credential file permission semantics" + ) + def test_auto_keyring_unavailable_enforces_file_permissions( + self, + mural_module: Any, + monkeypatch: pytest.MonkeyPatch, + tmp_path: pathlib.Path, + ) -> None: + _isolate_credential_env(monkeypatch, tmp_path) + + def _raise_unavailable(self: Any) -> None: + raise mural_module._KeyringUnavailable("test-induced") + + monkeypatch.setattr(mural_module.KeyringBackend, "__init__", _raise_unavailable) + + file_path = mural_module._resolve_credential_file("default", os.environ) + file_path.parent.mkdir(parents=True, exist_ok=True) + file_path.write_text("MURAL_CLIENT_ID=from-file\n", encoding="utf-8") + os.chmod(file_path, 0o644) + + with pytest.raises(mural_module.MuralError): + mural_module.resolve_backend("default") + def test_auto_fallback_warn_dedupes_per_profile_per_process( self, mural_module: Any, From 0033c76624138a58c827c23b6c2adc0f7ee31a1e Mon Sep 17 00:00:00 2001 From: akazlow Date: Tue, 23 Jun 2026 17:02:41 -0400 Subject: [PATCH 6/6] test(mural): avoid chmod in permission fallback test --- .../mural/tests/test_credential_storage.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/skills/experimental/mural/tests/test_credential_storage.py b/.github/skills/experimental/mural/tests/test_credential_storage.py index f1c017879..667b5ee2b 100644 --- a/.github/skills/experimental/mural/tests/test_credential_storage.py +++ b/.github/skills/experimental/mural/tests/test_credential_storage.py @@ -449,14 +449,27 @@ def _raise_unavailable(self: Any) -> None: monkeypatch.setattr(mural_module.KeyringBackend, "__init__", _raise_unavailable) + checked_paths: list[pathlib.Path] = [] + + def _raise_bad_perms(path: pathlib.Path, environ: dict[str, str]) -> None: + checked_paths.append(path) + raise mural_module.MuralError("simulated bad permissions") + + monkeypatch.setattr( + mural_module, + "_check_credential_file_perms", + _raise_bad_perms, + ) + file_path = mural_module._resolve_credential_file("default", os.environ) file_path.parent.mkdir(parents=True, exist_ok=True) file_path.write_text("MURAL_CLIENT_ID=from-file\n", encoding="utf-8") - os.chmod(file_path, 0o644) - with pytest.raises(mural_module.MuralError): + with pytest.raises(mural_module.MuralError, match="simulated bad permissions"): mural_module.resolve_backend("default") + assert checked_paths == [file_path] + def test_auto_fallback_warn_dedupes_per_profile_per_process( self, mural_module: Any,