fix(auth): treat empty / whitespace-only credentials as absent (closes #35)

[cmeans-claude-dev]

Summary

Closes #35. _resolve_credentials() in core/auth.py previously trusted that env / config / keyring values, if present, contained real credentials. Empty strings already fell through (Python truthiness handled them), but whitespace-only values like auth: {username: " "} mid-edit slipped through and reached login() as (' ', '\t', None), surfacing as a generic DSM 400 that pointed neither at the config nor at the empty values.

Reproducer (verified before fix)

cfg = AppConfig(..., auth=AuthConfig(username="   ", password="\t"), ...)
AuthManager(cfg, client)._resolve_credentials()
# Returns: ('   ', '\t', None)   ← bogus credentials flow into login()

After fix:

# Same input → AuthenticationError("No credentials found. Run 'mcp-synology setup'...")

Fix

New helper at the top of core/auth.py:

def _present_or_none(value: str | None) -> str | None:
    """Return value unchanged if it contains non-whitespace content; else None."""
    return value if (value and value.strip()) else None

Applied at all nine read sites — three credential fields (username, password, device_id) × three storage tiers (env vars, plaintext config file, OS keyring). The truthiness checks downstream (if not username and ...) are unchanged; the helper just normalizes empty / whitespace-only inputs to None at the boundary so they fall through the resolution chain like absent values.

Meaningful padding is preserved: a real password " pwd " keeps its surrounding spaces. Only purely empty / whitespace-only inputs are filtered ("".strip() == "" → falsy → filtered; " alice ".strip() == "alice" → truthy → preserved as " alice ").

Acceptance criteria (per #35)

• _resolve_credentials() validates username and password are non-empty before returning. Empty values are treated as "credentials absent" and fall through to the next strategy in the chain.
• Unit test for the empty-string case at each level of the resolution chain (env / config / keyring).
• Whitespace-only values also handled.
• CHANGELOG ### Fixed entry referencing this PR.

Test additions

Test	Strategy	Asserts
test_whitespace_config_credentials_fall_through	plaintext config	raises AuthenticationError
test_whitespace_env_credentials_fall_through	env vars	raises AuthenticationError
test_whitespace_keyring_credentials_fall_through	keyring	raises AuthenticationError
test_empty_string_env_credentials_fall_through	env (regression)	empty strings still fall through
test_whitespace_env_falls_through_to_valid_keyring	mixed	whitespace env doesn't shadow valid keyring
test_valid_credentials_with_internal_padding_preserved	config	" alice " is NOT stripped

Test plan

• uv run pytest — 518 passed (was 512; added 6 tests), 94 deselected, 96.15% coverage
• uv run pytest tests/core/test_auth.py::TestCredentialResolution — 10/10 pass (4 existing + 6 new)
• uv run ruff check src/ tests/ scripts/ — clean
• uv run ruff format --check src/ tests/ scripts/ — clean
• uv run mypy src/ scripts/ — clean
• CHANGELOG entry references this PR

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cmeans]

LGTM

[cmeans]

QA Round 1 — PR #66

Issue #35 acceptance-criteria audit

AC item	Status	Evidence
_resolve_credentials() validates username and password non-empty before returning; empty values fall through to next strategy	✅	_present_or_none() helper at auth.py:30-39; applied at all 9 read sites (env / config / keyring × user / pass / device_id)
Unit test for empty string at each level of resolution chain	✅	test_empty_string_env_credentials_fall_through (regression coverage); pre-existing tests already cover empty config
Whitespace-only handled	✅	test_whitespace_config_credentials_fall_through, test_whitespace_env_credentials_fall_through, test_whitespace_keyring_credentials_fall_through
CHANGELOG ### Fixed entry	✅	Present, references #66

Helper correctness audit

def _present_or_none(value: str | None) -> str | None:
    return value if (value and value.strip()) else None

Independent live behavior check:

Input	Output	Note
None	None	absent → absent
""	None	empty → absent
" "	None	spaces → absent
"\t", "\n"	None	other whitespace → absent
"alice"	"alice"	meaningful → preserved
" alice "	" alice "	padded password → preserved unchanged (not stripped)
"pwd-with-spaces "	"pwd-with-spaces "	trailing padding → preserved unchanged

The helper returns the original value, not value.strip() — meaningful padding in actual passwords is preserved. The strip is purely for the truthiness check. Correctly tested by test_valid_credentials_with_internal_padding_preserved.

Verification on 048ac50

Check	Result
uv run pytest	518 passed (was 512; +6 new), 94 deselected, 96.15% coverage (gate 95%)
uv run pytest tests/core/test_auth.py::TestCredentialResolution -v	10/10 pass — 4 pre-existing + 6 new
uv run ruff check src/ tests/ scripts/	clean
uv run ruff format --check src/ tests/ scripts/	69 files clean
uv run mypy src/ scripts/	clean (29 source files)
Required CI rollup (lint, typecheck, test 3.11/3.12/3.13, version-sync, validate-server-json)	all green

CI observation: vdsm flake

vdsm integration tests is showing FAILURE on this PR — but it's unrelated to the change here. The failing test is test_search_keyword_finds_directory, which creates a directory and waits for DSM's search indexer to pick it up:

INFO  Created search target directory: /testshare/Documents/Bambu Studio
INFO  search_files attempt 1/6: 0 results found
INFO  search_files attempt 2/6: 0 results found
... (6 retries with up to 15s waits) ...
FAILED tests/vdsm/test_vdsm_integration.py::TestSearch::test_search_keyword_finds_directory

The auth flow itself succeeded earlier in the run (INFO Authenticated as 'mcpadmin' (session: MCPSynology_vdsm-7-2-2_68c1a830)), and 46 of 47 vdsm tests passed — only the search-indexer test failed. No plausible connection to _present_or_none() in core/auth.py.

vdsm is not in the required-status-check ruleset and has continue-on-error: true (per #24's intentional setup until vdsm has a track record). Per the project's existing convention, this doesn't block merge. Recommend either:

• A. Re-run vdsm integration tests to confirm flake (one-click in Actions tab), or
• B. Accept as known DSM-search-indexer flakiness (consistent with #23's note that DSM search indexing in vdsm has historically been unreliable).

Verdict

Ready for QA Signoff on the code change itself. No findings on _present_or_none() or its application. CI observation above is for maintainer awareness — not a blocker since vdsm is continue-on-error and the failure is unrelated to this PR's diff. QA Approved remains the maintainer's call.

[cmeans]

QA audit — round 1

Branch / SHA: `fix/auth-validate-plaintext-credentials` @ `048ac50`

```
uv run pytest # 518 passed (was 512; +6), 94 deselected, 96.15%
uv run pytest tests/core/test_auth.py::TestCredentialResolution -v # 10/10 pass (4 + 6 new)
uv run ruff check src/ tests/ scripts/ # clean
uv run mypy src/ scripts/ # clean (29 source files)

Independent helper sanity (uv run python -c ...):

_present_or_none(None) = None

_present_or_none('') = None

_present_or_none(' ') = None

_present_or_none('\t') = None

_present_or_none('\n') = None

_present_or_none('alice') = 'alice'

_present_or_none(' alice ') = ' alice ' ← padding preserved

_present_or_none('pwd-with-spaces ') = 'pwd-with-spaces '

```

vdsm flake: `test_search_keyword_finds_directory` failed after 6 retries. Auth flow succeeded; failure is in DSM's post-auth search indexer. 46/47 vdsm tests passed; the one fail is unrelated to `_present_or_none()` in `core/auth.py`. `vdsm` is `continue-on-error: true` and not in the required-check ruleset, so the merge isn't blocked. Recommend maintainer either re-run vdsm or accept as known indexer flake.

Outcome: Ready for QA Signoff. All four issue #35 acceptance criteria met; no findings on the code change. `QA Approved` remains the maintainer's call.