diff --git a/src/cachekit/cache_handler.py b/src/cachekit/cache_handler.py index d25d188..c9a3d90 100644 --- a/src/cachekit/cache_handler.py +++ b/src/cachekit/cache_handler.py @@ -308,7 +308,7 @@ def __init__( deployment_uuid: Optional deployment-specific UUID for single-tenant mode. If not provided, uses env var or persistent file. master_key: Optional master key for encryption (hex-encoded). If not provided, - reads from REDIS_CACHE_MASTER_KEY environment variable. + reads from CACHEKIT_MASTER_KEY environment variable. enable_integrity_checking: Enable integrity checking (default: True) Uses xxHash3-64 (8 bytes) for all serializers. Set to False for @cache.minimal (speed-first, no checksums) diff --git a/src/cachekit/config/singleton.py b/src/cachekit/config/singleton.py index ba0af93..34cc4ca 100644 --- a/src/cachekit/config/singleton.py +++ b/src/cachekit/config/singleton.py @@ -1,5 +1,6 @@ """Thread-safe singleton pattern for global cachekit settings.""" +import os import threading from typing import Optional @@ -43,9 +44,25 @@ def get_settings() -> CachekitConfig: """ global _settings_instance - # Fast path - return cached instance without lock - if _settings_instance is not None: - return _settings_instance + # Fast path - snapshot the global once. A concurrent reset_settings() can null _settings_instance + # at any time, so the unlocked self-heal check below reads the snapshot, never the global, or it + # could dereference None between the not-None check and the .master_key access. + instance = _settings_instance + if instance is not None: + # Self-heal the keyless-then-key-set ordering trap (#195): if the config was first built + # before CACHEKIT_MASTER_KEY entered the environment (e.g. an import-time cache decorator + # evaluated before the app loaded its secrets), it froze master_key=None — encryption would + # then silently never activate. Re-read once the key appears, so it turns on without an + # explicit reset_settings(). Idempotent: after the rebuild master_key is set, so this never + # fires again (no per-call churn once a key is present). + if instance.master_key is None and os.environ.get("CACHEKIT_MASTER_KEY"): + with _settings_lock: + # Re-read the global under the lock: a peer may have rebuilt it (key now set) or + # reset_settings() may have cleared it (back to None). Rebuild only if still keyless. + if _settings_instance is None or _settings_instance.master_key is None: + _settings_instance = CachekitConfig.from_env() + return _settings_instance + return instance # Slow path - create instance with lock with _settings_lock: diff --git a/src/cachekit/serializers/encryption_wrapper.py b/src/cachekit/serializers/encryption_wrapper.py index 35b6613..3c7eb73 100644 --- a/src/cachekit/serializers/encryption_wrapper.py +++ b/src/cachekit/serializers/encryption_wrapper.py @@ -128,7 +128,7 @@ def _setup_encryption(self, master_key: Optional[bytes]) -> None: settings = get_settings() if not settings.master_key: raise EncryptionError( - "Master key required. Set REDIS_CACHE_MASTER_KEY environment variable or pass master_key parameter." + "Master key required. Set CACHEKIT_MASTER_KEY environment variable or pass master_key parameter." ) try: master_key = bytes.fromhex(settings.master_key.get_secret_value()) diff --git a/tests/unit/test_encryption_config_resolution.py b/tests/unit/test_encryption_config_resolution.py new file mode 100644 index 0000000..b421e61 --- /dev/null +++ b/tests/unit/test_encryption_config_resolution.py @@ -0,0 +1,61 @@ +"""Encryption-config resolution correctness. + +#195: get_settings() built the config once and cached it process-wide. If the first build happened +before CACHEKIT_MASTER_KEY entered the environment (e.g. an import-time @cache decorator before the +app loads secrets), it froze master_key=None forever — encryption then silently never activated. +The singleton must re-read once the key appears. + +#194: the missing-key error pointed users at REDIS_CACHE_MASTER_KEY, which is never read; the only +honored variable is CACHEKIT_MASTER_KEY. +""" + +from __future__ import annotations + +import pytest + +from cachekit.config.singleton import get_settings, reset_settings +from cachekit.serializers.auto_serializer import AutoSerializer +from cachekit.serializers.encryption_wrapper import EncryptionError, EncryptionWrapper + +_HEX_KEY = "a" * 64 # 32-byte hex master key + + +@pytest.mark.unit +class TestKeylessSingletonSelfHeals: + """#195: a keyless singleton must not freeze permanently once the key is set.""" + + def test_get_settings_rereads_when_master_key_appears(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("CACHEKIT_MASTER_KEY", raising=False) + reset_settings() + try: + assert get_settings().master_key is None # first build: env not yet set -> keyless + monkeypatch.setenv("CACHEKIT_MASTER_KEY", _HEX_KEY) + assert get_settings().master_key is not None # must self-heal (the bug froze this at None) + finally: + reset_settings() + + def test_get_settings_stable_once_key_present(self, monkeypatch: pytest.MonkeyPatch) -> None: + # Guard: self-heal must not re-read on every call once the key is loaded. + monkeypatch.setenv("CACHEKIT_MASTER_KEY", _HEX_KEY) + reset_settings() + try: + assert get_settings() is get_settings() + finally: + reset_settings() + + +@pytest.mark.unit +class TestMissingKeyErrorNamesCorrectEnvVar: + """#194: the missing-key error must name the env var that is actually honored.""" + + def test_error_names_cachekit_master_key(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("CACHEKIT_MASTER_KEY", raising=False) + reset_settings() + try: + with pytest.raises(EncryptionError) as exc: + EncryptionWrapper(serializer=AutoSerializer(), master_key=None) + msg = str(exc.value) + assert "CACHEKIT_MASTER_KEY" in msg + assert "REDIS_CACHE_MASTER_KEY" not in msg + finally: + reset_settings()