From da81c09b6e418469e21ebd42fb961f87a57afd54 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sun, 21 Jun 2026 11:00:05 +1000 Subject: [PATCH 1/2] fix: re-read settings when CACHEKIT_MASTER_KEY appears + correct missing-key env var name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #195: get_settings() cached the config once; if first built before CACHEKIT_MASTER_KEY was in the environment (e.g. an import-time cache decorator before secrets load), it froze master_key=None forever — encryption then silently never activated. get_settings() now re-reads once the key appears (under the existing lock; idempotent, no churn after). #194: the missing-key error and a docstring named REDIS_CACHE_MASTER_KEY, which is never read; corrected both to CACHEKIT_MASTER_KEY (the only honored var), matching config/nested.py. Closes #194. Closes #195. --- src/cachekit/cache_handler.py | 2 +- src/cachekit/config/singleton.py | 13 ++++ .../serializers/encryption_wrapper.py | 2 +- .../unit/test_encryption_config_resolution.py | 61 +++++++++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_encryption_config_resolution.py 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..2ef0dcb 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 @@ -45,6 +46,18 @@ def get_settings() -> CachekitConfig: # Fast path - return cached instance without lock if _settings_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 _settings_instance.master_key is None and os.environ.get("CACHEKIT_MASTER_KEY"): + with _settings_lock: + # Re-check under the lock (reset_settings() also locks): a peer thread may have + # already rebuilt with the key, in which case master_key is no longer None. + if _settings_instance.master_key is None: + _settings_instance = CachekitConfig.from_env() return _settings_instance # Slow path - create instance with 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() From e22c3f7451b965c3a03f64f828bafc3af7eccaf6 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sun, 21 Jun 2026 11:09:08 +1000 Subject: [PATCH 2/2] fix: snapshot settings singleton before the self-heal check (avoid concurrent-reset AttributeError) Addresses CodeRabbit on #200: the #195 self-heal dereferenced _settings_instance.master_key in the unlocked fast path after a non-None check. A concurrent reset_settings() between the two could null the global and AttributeError (or return None). Snapshot the global to a local for the unlocked checks; re-read it under the lock (rebuilding if a peer reset or it is still keyless). --- src/cachekit/config/singleton.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/cachekit/config/singleton.py b/src/cachekit/config/singleton.py index 2ef0dcb..34cc4ca 100644 --- a/src/cachekit/config/singleton.py +++ b/src/cachekit/config/singleton.py @@ -44,21 +44,25 @@ def get_settings() -> CachekitConfig: """ global _settings_instance - # Fast path - return cached instance without lock - if _settings_instance is not None: + # 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 _settings_instance.master_key is None and os.environ.get("CACHEKIT_MASTER_KEY"): + if instance.master_key is None and os.environ.get("CACHEKIT_MASTER_KEY"): with _settings_lock: - # Re-check under the lock (reset_settings() also locks): a peer thread may have - # already rebuilt with the key, in which case master_key is no longer None. - if _settings_instance.master_key is None: + # 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 _settings_instance + return instance # Slow path - create instance with lock with _settings_lock: