fix(auth): narrow keyring exception handler + log root cause

[cmeans-claude-dev]

Summary

Closes #38. The bare except Exception keyring handler in core/auth.py:147-148 collapsed every failure mode into the same useless "Keyring not available." debug line. Now narrows to typed handlers (KeyringError, OSError) with exc_info=True, and bumps the D-Bus socket-missing pre-check from DEBUG to INFO with three concrete remediations.

Behavior change

Before	After
except Exception: logger.debug("Keyring not available.") swallows everything	except KeyringError as e: logger.debug("Keyring access failed: %s", e, exc_info=True) + except OSError as e: logger.debug("Keyring OS-level error: %s", e, exc_info=True)
mcp-synology check -v shows generic "Keyring not available." regardless of cause	-v shows the actual exception class, message, and traceback
Library bugs (e.g. an AttributeError from a corrupted keyring backend) silently swallowed	Library bugs propagate so they can be triaged
Linux headless service + missing D-Bus socket → DEBUG-only "D-Bus socket not found at %s; keyring may not work"	INFO-level: names the socket path AND points at three concrete remediations (real desktop session, dbus-run-session, SYNOLOGY_* env vars)

The defense-in-depth property is preserved: a keyring blow-up still falls through to the next strategy in the resolver chain (config-file or env credentials), so a working auth.username/password configuration continues to work even when the keychain is locked.

Coverage of typed errors

keyring.errors.KeyringError is the base class. The hierarchy this catches:

• NoKeyringError — common on headless hosts with no backend installed
• KeyringLocked — operator-actionable on macOS (unlock the keychain)
• InitError — backend installed but couldn't initialize (e.g. D-Bus reach failure surfaced via the keyring layer)
• PasswordSetError / PasswordDeleteError — write-path errors (we only read here, but these descend from KeyringError and are correctly caught for free)

OSError covers the D-Bus-socket-present-but-unreachable variant separately, which the keyring library re-raises as a raw OSError rather than wrapping in an InitError.

QA

Manual tests

1. • Run uv run pytest — 553 passed at 96.13% coverage.
2. • Run uv run pytest tests/core/test_auth.py::TestKeyringErrorHandling -v — 3 passed.
3. • Run uv run pytest tests/core/test_auth.py::TestDbusSocketMissing -v — 1 passed; the strengthened assertion now requires the INFO-level remediation hint.
4. • Run uv run ruff check src/ tests/ scripts/ — clean.
5. • Run uv run ruff format --check src/ tests/ scripts/ — clean.
6. • Run uv run mypy src/ scripts/ — clean.
7. • Spot-check git grep -n 'except Exception' src/mcp_synology/core/auth.py — only the dispatch-loop except Exception as e: # noqa: BLE001 — best-effort dispatch remains (that one is intentional, lives inside the on-reauth callback fan-out).
8. • On a Linux box without an active D-Bus session, SYNOLOGY_LOG_LEVEL=info mcp-synology check should surface the new INFO line "D-Bus socket not found at /run/user//bus; OS keyring is unavailable. Run 'mcp-synology setup' from a real desktop session..." instead of falling through silently to "no credentials".

Verification I already ran

Check	Result
uv run pytest	553 passed, 100 deselected, 96.13% coverage
uv run ruff check src/ tests/ scripts/	clean
uv run ruff format --check src/ tests/ scripts/	72 files already formatted
uv run mypy src/ scripts/	clean (30 files, strict-mode)
Discovery: python -c "import keyring.errors; print([n for n in dir(keyring.errors) if not n.startswith('_')])"	confirms KeyringError is the base class for all typed keyring failures (KeyringLocked, NoKeyringError, InitError, PasswordSetError, PasswordDeleteError)

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cmeans]

LGTM

[github-actions]

New commits pushed while QA was active. QA review invalidated — resetting to Awaiting CI.

[cmeans-claude-dev]

Rebased on main after PR #79 merge

PR #79 (mcp-publisher v1.5.0 → v1.7.6) just merged into main, so this branch picked up a CHANGELOG conflict on the shared ## Unreleased > ### Fixed section. Rebased and force-pushed fix/keyring-handler-narrow onto the new main:

• CHANGELOG entries from fix(workflow): bump mcp-publisher v1.5.0 → v1.7.6 for registry OIDC audience #79 and this PR (fix(auth): narrow keyring exception handler + log root cause #80) are now both present under ## Unreleased > ### Fixed, in merge order.
• The chore(changelog): pin #38 entry to PR #80 follow-up commit was redundant after the rebase (the entry was rewritten in-place during conflict resolution and already references fix(auth): narrow keyring exception handler + log root cause #80) — skipped during git rebase --continue so the branch is now a single clean commit.
• No code or test changes from the rebase; only CHANGELOG.

Force-pushed with --force-with-lease to be safe. Verification on the rebased head matches the pre-rebase verification: 553 unit tests pass at 96.13% coverage, ruff/format/mypy clean.

[cmeans]

LGTM

[cmeans]

QA Round 1 — PASS

Closes the keyring-handler narrowing I called out as a real-impact security item back in the PR #74 (SafeSkill) review. Clean fix shape, comprehensive tests, defense-in-depth preserved.

Issue #38 acceptance criteria

AC	Coverage
Narrow except to (KeyringError, OSError)	✓ Two separate typed handlers at auth.py:158-175
Log actual exception at DEBUG with exc_info=True	✓ Both branches; tests assert record.exc_info is not None, not just message presence
INFO-level hint when D-Bus socket missing	✓ auth.py:131-140 rewritten from DEBUG to INFO with three concrete remediations (real desktop session, dbus-run-session, SYNOLOGY_* env vars)
Unit test: KeyringError logged with message, not silently swallowed	✓ TestKeyringErrorHandling::test_keyring_error_logged_with_exc_info_at_debug plus a regression assertion that the legacy flat "Keyring not available." message does NOT appear
CHANGELOG ### Fixed entry	✓ Present, #80-linked, accurate test count and coverage figure

Verification

Check	Result
uv run pytest	553 passed (+3 new TestKeyringErrorHandling), 100 deselected, 96.13% coverage
Targeted TestKeyringErrorHandling + TestDbusSocketMissing	4/4 pass directly
uv run ruff check src/ tests/ scripts/	clean
uv run ruff format --check src/ tests/ scripts/	72 files already formatted
uv run mypy src/ scripts/	clean (30 files, strict-mode)
grep -n 'except Exception' src/mcp_synology/core/auth.py	only auth.py:282 (the on-reauth callback dispatch loop with # noqa: BLE001 — best-effort dispatch, QA-approved on #73). The pre-fix auth.py:147-148 bare-except is gone.
Required CI on ec630b3	13/13 green (vdsm completed SUCCESS)

Test-quality notes

A few things I want to call out as good (not findings):

• The _no_keyring() fixture update — bumping the side-effect from bare Exception to keyring.errors.NoKeyringError — keeps the production-shaped error path exercised in every existing test that uses the fixture. Without this, the new typed handlers would only be exercised in the three new cases and the rest of the suite would test against an unrealistic error type.
• test_keyring_failure_does_not_block_config_credentials is the right defense-in-depth check: the bare-except previously had a "feature" (silent swallow → fall through) that the narrow handlers must preserve. Tests prove they do.
• The strengthened test_dbus_not_set_when_socket_missing_on_linux asserts the INFO message contains the socket path AND mcp-synology setup AND SYNOLOGY_USERNAME — three independent strings — so a partial regression (e.g. the remediation hint truncated) would still fail the test. That's stronger than what AC#3 strictly requires.
• Manual test #8 is a Linux-box verification of the operator-visible INFO line; the strengthened unit test asserts the exact output that would surface, so #8 is implicitly covered without needing to actually deploy the build to a headless Linux service.

PR-body manual tests 1–8 all checked.

Verdict

Ready for QA Signoff. Final maintainer call.

Side note: this is the third of the security-relevant items I listed in the PR #74 SafeSkill review (after #69/#71 atomic writes and #66 whitespace credential filtering already merged). #46 supply-chain hardening, #40 version-string validation, #39 background update-check error visibility, #44 PyPI gate are still open if the security-posture work is on a roll.

[cmeans]

Applying Ready for QA Signoff as the final act of round 1. All 5 #38 acceptance criteria satisfied; 4/4 targeted tests pass; defense-in-depth fallback chain confirmed; only legitimate except Exception remaining in auth.py is the on-reauth dispatch loop (QA-approved on #73). 553/553, 96.13%, ruff/mypy clean, 13/13 required CI green on ec630b3 (the post-rebase head). Final maintainer call.