security: validate OS keyring backend and harden sanity check

[brandwe]

Summary

• Fail closed when keyring selects anything other than the canonical OS backend for macOS, Linux, or Windows.
• Add InsecureKeyringBackendError with the active backend class in the message for clear operator diagnostics.
• Harden keyring_sanity.check() to validate the backend before writing, return backend telemetry, and use per-call random probe keys.

Security impact

Without this guard, Linux headless/container/sudo sessions can silently fall through to keyrings.alt plaintext or weak file backends, exposing the ADR-003 Blueprint private key. The new allowlist is backwards-compatible for operators on canonical OS keystores and raises only for misconfigured/insecure installs.

Tests

• Added 17 tests (1341 -> 1358 collected).
• Verified: pytest -v --tb=short && ruff check .

[brandwe]

Opus review (claude-opus-4.7) — VERDICT: REQUEST_CHANGES

Independent cross-model review against the diff (PR authored by gpt-5.5).

The runtime check is correct, but two paths bypass it that defeat the threat model:

🔴 HIGH — Provisioning paths bypass the check

scripts/setup.sh (~line 996) and scripts/entra_provisioning.py:_keychain_store_cert (L80-L107) call keyring.set_password('entrabot', 'blueprint-private-key', pem) and equivalents with no backend assertion. On a host where keyring.get_keyring() returns keyrings.alt.file.PlaintextKeyring, the Blueprint private key (and the Provisioner cert+key) get written to disk in cleartext at provisioning time. The MCP server's new constructor check then fails loudly on the next boot — but by then the secret is already compromised on disk.

The runtime error doesn't tell the operator 'rotate now — your key was written to ~/.local/share/python_keyring/keyring_pass.cfg in cleartext.'

Fix: Call assert_allowed_keyring_backend() before every keyring write in entra_provisioning._keychain_store_cert and the inline Python in setup.sh. The setup-script call will need a small importable shim since it currently runs python -c inline.

🔴 HIGH — tools/audit.py swallows InsecureKeyringBackendError

src/entrabot/tools/audit.py L46-L53:

if agent_id is None:
    try:
        from entrabot.platform import get_credential_store
        store = get_credential_store()  # raises InsecureKeyringBackendError
        agent_id = store.retrieve('entrabot', 'active_client_id') or 'unknown'
    except Exception:
        agent_id = 'unknown'

Bare except Exception catches the new error. Reproduced by reviewer: patched keyring.get_keyring to a non-allowlisted backend, called log_event(...), got back agent_id='unknown' with no exception and the audit file written normally. The defense-in-depth signal vanishes the first time anything audits.

Fix: Narrow the except — let InsecureKeyringBackendError (and probably any EntraBotError) propagate; only swallow the legitimate 'no active_client_id provisioned yet' cases.

🟡 MEDIUM — keyring.backends.libsecret.Keyring missing from Linux allowlist

libsecret is shipped by keyring itself (not keyrings.alt), talks to the same Secret Service collection as SecretService.Keyring, and is the recommended default on Fedora/openSUSE. Operators with default-keyring=keyring.backends.libsecret.Keyring in keyringrc.cfg will now get InsecureKeyringBackendError at startup — a false-positive regression.

Fix: Add 'keyring.backends.libsecret.Keyring' to the Linux tuple.

🟡 MEDIUM — No escape hatch for operator-supplied secure backends

The allowlist is hard-coded with no ENTRABOT_KEYRING_BACKEND_ALLOWLIST env override, and the error message doesn't include remediation guidance (precedent: MissingPermissionError, RemovedModeError).

🟢 LOW — keyring_sanity.check() is not called from any production code

grep -rn keyring_sanity src/ scripts/ returns only the module + tests. The hardening here is correct but operationally inert until something (preflight?) actually calls it.

Missing test coverage

• No test that audit.log_event surfaces InsecureKeyringBackendError (tests/tools/test_audit.py mocks the credential store entirely)
• No test for the fail.Keyring rejection on macOS/Windows
• No test for the libsecret-allowlisted case
• tests/test_keyring_sanity.py:13 monkeypatches assert_allowed_keyring_backend with raising=False even in happy-path tests — so a future rename of the function would silently pass

The runtime portion of this PR is solid. Blocking on the two HIGH gaps because together they let the threat model the PR claims to close happen anyway (key written in cleartext at provisioning; error muted at first audit).

[brandwe]

Follow-up commit e21ca62 addresses the 3 review blockers:

1. ✅ tools/audit.log_event no longer swallows InsecureKeyringBackendError — bare except Exception is now narrowed to except InsecureKeyringBackendError: raise; except Exception: agent_id = 'unknown'. New test: test_insecure_keyring_backend_error_propagates (and test_unrelated_credential_store_error_falls_back_to_unknown to lock in the forgiveness path for unrelated cases).
2. ✅ Provisioning paths now assert the backend before any keyring read/write — scripts/entra_provisioning._keychain_{store,get,delete}_cert and the inline python in scripts/setup.sh (Blueprint cert generation, around line 996) call assert_allowed_keyring_backend() first. On Windows the assertion is a no-op since Windows uses the file-backed cert store with explicit ACLs, not the keyring. New file: tests/test_entra_provisioning_keyring_backend.py with 3 tests covering store, get, and the happy path.
3. ✅ keyring.backends.libsecret.Keyring added to the Linux allowlist — libsecret is shipped by keyring itself (not keyrings.alt), is the recommended default on Fedora / openSUSE, and talks to the same Secret Service collection as the existing SecretService backend. Previously a false-positive rejection at startup. Extended the existing parametrize.

Suite: 1358 → 1366 passing (8 new tests, +2 audit, +3 provisioning, +1 libsecret-accepted, +1 mac/windows fail.Keyring symmetry, +1 typo correction). ruff check . green.

The MEDIUM/LOW concerns (no ENTRABOT_KEYRING_BACKEND_ALLOWLIST override, no remediation pointer in error message, keyring_sanity.check() not yet wired into preflight, read-side script gaps) are not addressed in this commit — happy to do them in a follow-up PR if you want them in scope here.