fix(bindings): warn when Python SecureStorage gets keyring's failing or null backend

CHANGELOG.md

@@ -432,6 +432,16 @@ archived by series under [docs/changelog/](docs/changelog/); see the
   secure provider in the UDL and the integration guide, not only on the
   protocol-state provider: a custom secure store that reports a transient
   fault as `CorruptedData` loses every sealed protocol-state record.
+- **Python `SecureStorage` warns on a host without a secret service.** Its
+  warning about a backend that cannot hold MLS keys checked only the class name,
+  and keyring's failing and null backends are both classes named `Keyring`, so a
+  headless Linux host or container (which gets `keyring.backends.fail.Keyring`)
+  was never warned. The check now also recognises those modules, and judges the
+  first backend inside keyring's `ChainerBackend` (the one writes go to) rather
+  than the chainer itself, which no check matched. The warning names the
+  backend in full (`keyring.backends.fail.Keyring`) and points a headless host
+  at the built-in file stores (`store_key`). Log output only; storage behaviour
+  is unchanged.
 - **Frames a bridge builds for the core carried a fixed application id.** The
   iOS, Android and Python bridges stamped `"offline-messenger"` on every relay
   answer, relay group frame and legacy plain-text DM they rebuilt for the

bindings/python/README.md

@@ -167,9 +167,10 @@ MLS cryptographic key material is stored using the `keyring` library:
 | Linux | Secret Service (GNOME Keyring / KWallet) |
 | Windows | Windows Credential Locker |
 
-On a host with none of those available, `keyring` falls back to a null or
-plaintext backend. `SecureStorage` logs a warning when it detects one, but it
-does not refuse to run — and on Python that warning is louder than it looks.
+On a host with none of those available, `keyring` falls back to its failing
+backend (every call raises), or to a plaintext one if `keyrings.alt` is
+installed. `SecureStorage` logs a warning when it detects either, but it
+does not refuse to run, and on Python that warning is louder than it looks.
 The credential store also holds `protocol_state_record_key`, the per-install key
 that seals pending messages, outbox entries, and media descriptors before they
 reach `AppStateStorage`. On a plaintext backend that key sits in a readable

bindings/python/offline_protocol_sdk/secure_storage.py

@@ -14,13 +14,17 @@
 import threading
 
 import keyring
+import keyring.backends.chainer
 
 from . import legacy_store_adoption
 from .offline_protocol import MlsStorageError, MlsStorageProvider
 from .storage_namespace import require_account_storage_namespace
 
 logger = logging.getLogger(__name__)
 
+#: keyring backends that cannot hold a secret: every call fails, or nothing is kept.
+_INSECURE_KEYRING_MODULES = frozenset({"keyring.backends.fail", "keyring.backends.null"})
+
 # Base service name used for keyring entries
 _DEFAULT_SERVICE = "offline-protocol-mls-v2"
 
@@ -96,16 +100,43 @@ def __init__(
                 "ProtocolManager build the provider."
             )
 
-        # Warn if keyring resolved to a plaintext or null backend — MLS key
-        # material would be stored unprotected.
+        # Warn if keyring resolved to a failing, null or plaintext backend: MLS
+        # key material would not be stored, or not stored securely. keyring's
+        # failing and null backends are both classes named plain ``Keyring``
+        # (in ``keyring.backends.fail`` and ``keyring.backends.null``), so the
+        # module is checked as well as the class name; the failing one is what a
+        # host without a secret service gets. When two or more backends are
+        # viable, keyring hands back a ``ChainerBackend`` instead, and writes go
+        # to the first backend it chains, so that is the backend judged. (A host
+        # whose only viable backend is ``keyrings.alt``'s plaintext file gets
+        # that backend directly, not a chain; the unwrap is for a chain that
+        # happens to rank an insecure backend first.)
         try:
             backend = keyring.get_keyring()
-            backend_name = type(backend).__name__
-            if "Fail" in backend_name or "Null" in backend_name or "PlaintextKeyring" in backend_name:
+            # A chain with nothing in it stores nothing, like the null backend.
+            empty_chain = False
+            if isinstance(backend, keyring.backends.chainer.ChainerBackend):
+                chained = backend.backends
+                empty_chain = not chained
+                if chained:
+                    backend = chained[0]
+            backend_cls = type(backend)
+            backend_name = f"{backend_cls.__module__}.{backend_cls.__name__}"
+            if (
+                empty_chain
+                or backend_cls.__module__ in _INSECURE_KEYRING_MODULES
+                # keyring's own failing and null backends are caught by module
+                # above; the name markers catch third-party backends.
+                or "Fail" in backend_cls.__name__
+                or "Null" in backend_cls.__name__
+                or "PlaintextKeyring" in backend_cls.__name__
+            ):
                 logger.warning(
-                    "keyring backend is '%s' — MLS keys will NOT be stored "
+                    "keyring backend is '%s': MLS keys will NOT be stored "
                     "securely. Install a platform secret service (e.g. "
-                    "gnome-keyring, kwallet) for production use.",
+                    "gnome-keyring, kwallet), or on a headless host pass "
+                    "store_key or store_key_env to ProtocolManager to use the "
+                    "built-in file stores.",
                     backend_name,
                 )
         except Exception:

bindings/python/tests/test_secure_storage_backend_warning.py

@@ -0,0 +1,114 @@
+"""SecureStorage warns when keyring resolved to a backend that cannot hold secrets.
+
+On a host without a secret service (a headless Linux server, a container), the
+keyring library selects ``keyring.backends.fail.Keyring``, whose every call
+raises. Both it and ``keyring.backends.null.Keyring`` are classes named just
+``Keyring``, so a check on the class name alone never recognised the case the
+warning exists for. With two or more viable backends keyring returns a
+``ChainerBackend`` instead, and the backend that matters is the first one it
+chains, since writes go there. (A host whose only viable backend is
+``keyrings.alt``'s plaintext file gets that backend directly; the chain cases
+here are defensive.)
+"""
+
+from __future__ import annotations
+
+import logging
+
+import keyring
+import keyring.backends.chainer
+import keyring.backends.fail
+import keyring.backends.null
+import pytest
+
+from offline_protocol_sdk.secure_storage import SecureStorage
+from offline_protocol_sdk.storage_namespace import account_storage_namespace
+
+NAMESPACE = account_storage_namespace("backend-warning-test", "p")
+
+
+def _plaintext_backend():
+    # keyrings.alt is not a dependency; its file backend is a class of this name.
+    return type("PlaintextKeyring", (keyring.backends.null.Keyring,), {"__module__": "keyrings.alt.file"})()
+
+
+def _named(name):
+    # A third-party backend: only the class name marks it as insecure.
+    return lambda: type(name, (keyring.backends.null.Keyring,), {"__module__": "keyrings.vendor"})()
+
+
+def _platform_backend():
+    # A real secret service backend: class name and module name carry no marker.
+    return type("Keyring", (keyring.backends.null.Keyring,), {"__module__": "keyring.backends.macOS"})()
+
+
+def _chain(*backends):
+    # ``backends`` is a classproperty that discovers what is installed; pin it.
+    return type("ChainerBackend", (keyring.backends.chainer.ChainerBackend,),
+                {"__module__": "keyring.backends.chainer", "backends": list(backends)})()
+
+
+def _plaintext_chain():
+    return _chain(_plaintext_backend(), keyring.backends.null.Keyring())
+
+
+def _platform_chain():
+    return _chain(_platform_backend(), _plaintext_backend())
+
+
+def _qualified(instance) -> str:
+    cls = type(instance)
+    return f"{cls.__module__}.{cls.__name__}"
+
+
+@pytest.fixture
+def backend(monkeypatch: pytest.MonkeyPatch):
+    def use(instance):
+        monkeypatch.setattr(keyring, "get_keyring", lambda: instance)
+        return instance
+
+    return use
+
+
+def _warnings(caplog) -> list[str]:
+    return [r.getMessage() for r in caplog.records
+            if r.levelno >= logging.WARNING and "will NOT be stored securely" in r.getMessage()]
+
+
+@pytest.mark.parametrize("make", [keyring.backends.fail.Keyring, keyring.backends.null.Keyring,
+                                  _plaintext_backend, _named("FailKeyring"), _named("NullKeyring")],
+                         ids=["fail", "null", "plaintext", "third_party_fail", "third_party_null"])
+def test_insecure_backend_is_warned_about(backend, caplog, make):
+    instance = backend(make())
+    with caplog.at_level(logging.WARNING, logger="offline_protocol_sdk.secure_storage"):
+        SecureStorage(namespace=NAMESPACE, adopt_legacy_store=False)
+    messages = _warnings(caplog)
+    assert len(messages) == 1, caplog.text
+    assert f"'{_qualified(instance)}'" in messages[0]
+    assert "store_key" in messages[0]
+
+
+def test_chain_led_by_an_insecure_backend_is_warned_about(backend, caplog):
+    chain = backend(_plaintext_chain())
+    with caplog.at_level(logging.WARNING, logger="offline_protocol_sdk.secure_storage"):
+        SecureStorage(namespace=NAMESPACE, adopt_legacy_store=False)
+    messages = _warnings(caplog)
+    assert len(messages) == 1, caplog.text
+    assert f"'{_qualified(chain.backends[0])}'" in messages[0]
+
+
+def test_empty_chain_is_warned_about(backend, caplog):
+    backend(_chain())
+    with caplog.at_level(logging.WARNING, logger="offline_protocol_sdk.secure_storage"):
+        SecureStorage(namespace=NAMESPACE, adopt_legacy_store=False)
+    messages = _warnings(caplog)
+    assert len(messages) == 1, caplog.text
+    assert "'keyring.backends.chainer.ChainerBackend'" in messages[0]
+
+
+@pytest.mark.parametrize("make", [_platform_backend, _platform_chain], ids=["platform", "chain"])
+def test_platform_backend_is_not_warned_about(backend, caplog, make):
+    backend(make())
+    with caplog.at_level(logging.WARNING, logger="offline_protocol_sdk.secure_storage"):
+        SecureStorage(namespace=NAMESPACE, adopt_legacy_store=False)
+    assert _warnings(caplog) == []