From c6aa23c1c973e15ed6ce3f84c8b067b4db03d004 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 28 Mar 2026 17:59:48 +1100 Subject: [PATCH 1/2] fix: fall back to CACHEKIT_MASTER_KEY env var in cache.secure decorator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #69 — cache.secure short-circuited with ValueError before reaching the pydantic-settings config layer that resolves CACHEKIT_MASTER_KEY from the environment. Now uses get_settings().master_key as fallback before raising, consistent with how the nested config layer already works. --- src/cachekit/decorators/intent.py | 10 +++++-- tests/docs/test_decorator_api.py | 24 ++++++++++----- tests/test_decorator_intent_examples.py | 39 +++++++++++++++++++++---- 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/cachekit/decorators/intent.py b/src/cachekit/decorators/intent.py index 4837004..3d71ef9 100644 --- a/src/cachekit/decorators/intent.py +++ b/src/cachekit/decorators/intent.py @@ -148,11 +148,17 @@ def decorator(f: F) -> F: elif _intent == "production": # Renamed from "safe" resolved_config = DecoratorConfig.production(backend=backend, **manual_overrides) elif _intent == "secure": - # Extract master_key from manual_overrides (required for secure preset) + # Extract master_key from manual_overrides, fall back to env var via settings master_key = manual_overrides.pop("master_key", None) tenant_extractor = manual_overrides.pop("tenant_extractor", None) or None if not master_key: - raise ValueError("cache.secure requires master_key parameter") + from cachekit.config.singleton import get_settings + + settings_key = get_settings().master_key + if settings_key: + master_key = settings_key.get_secret_value() + if not master_key: + raise ValueError("cache.secure requires master_key parameter or CACHEKIT_MASTER_KEY environment variable") resolved_config = DecoratorConfig.secure( master_key=master_key, tenant_extractor=tenant_extractor, backend=backend, **manual_overrides ) diff --git a/tests/docs/test_decorator_api.py b/tests/docs/test_decorator_api.py index cdd8344..17267e8 100644 --- a/tests/docs/test_decorator_api.py +++ b/tests/docs/test_decorator_api.py @@ -73,11 +73,19 @@ def test_func(): result = test_func() assert result == "test", f"Preset {preset} broke function execution" - def test_secure_preset_requires_master_key(self): - """@cache.secure requires master_key parameter.""" - # Verify secure preset exists but requires master_key - with pytest.raises(ValueError, match="master_key"): - - @cache.secure - def test_func(): - return "test" + def test_secure_preset_requires_master_key(self, monkeypatch): + """@cache.secure requires master_key parameter or env var.""" + from cachekit.config.singleton import reset_settings + + monkeypatch.delenv("CACHEKIT_MASTER_KEY", raising=False) + reset_settings() + + try: + # Verify secure preset exists but requires master_key + with pytest.raises(ValueError, match="master_key"): + + @cache.secure + def test_func(): + return "test" + finally: + reset_settings() diff --git a/tests/test_decorator_intent_examples.py b/tests/test_decorator_intent_examples.py index 6a67aed..f536240 100644 --- a/tests/test_decorator_intent_examples.py +++ b/tests/test_decorator_intent_examples.py @@ -124,13 +124,40 @@ def test_secure_config_monitoring_enabled(self): assert config.monitoring.collect_stats is True assert config.monitoring.enable_tracing is True - def test_secure_decorator_requires_master_key(self): - """@cache.secure decorator requires master_key parameter.""" - with pytest.raises(ValueError, match="master_key"): + def test_secure_decorator_requires_master_key(self, monkeypatch): + """@cache.secure raises when no master_key param AND no env var.""" + from cachekit.config.singleton import reset_settings - @cache.secure(backend=None) # Missing master_key in secure context - def secure_func(): - pass + monkeypatch.delenv("CACHEKIT_MASTER_KEY", raising=False) + reset_settings() + + try: + with pytest.raises(ValueError, match="master_key"): + + @cache.secure(backend=None) # No master_key param, no env var + def secure_func(): + pass + finally: + reset_settings() + + def test_secure_decorator_reads_master_key_from_env(self, monkeypatch): + """@cache.secure falls back to CACHEKIT_MASTER_KEY env var (issue #69).""" + from cachekit.config.singleton import reset_settings + + test_key = "ab" * 32 # 64 hex chars = 32 bytes + monkeypatch.setenv("CACHEKIT_MASTER_KEY", test_key) + reset_settings() # Force re-read from env + + try: + + @cache.secure(backend=None, ttl=300) + def secure_from_env(x: int) -> int: + return x * 2 + + # Should NOT raise - master_key resolved from env + assert secure_from_env is not None + finally: + reset_settings() def test_secure_decorator_with_tenant_extractor(self): """Secure preset supports multi-tenant extraction.""" From d201ac5c8e4ec79f76f53ed77d6e0042ebd8251a Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 28 Mar 2026 18:36:21 +1100 Subject: [PATCH 2/2] test: add critical path tests for cache.secure env var fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coverage for the fix in intent.py — CI measures coverage only on tests/critical/ for PRs, so these need to live here. --- tests/critical/conftest.py | 12 +++- tests/critical/test_secure_env_fallback.py | 66 ++++++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 tests/critical/test_secure_env_fallback.py diff --git a/tests/critical/conftest.py b/tests/critical/conftest.py index 1452b76..b4f0d34 100644 --- a/tests/critical/conftest.py +++ b/tests/critical/conftest.py @@ -6,6 +6,12 @@ def pytest_runtest_setup(item): """Skip redis setup for file backend and cachekitio metrics tests.""" - if "file_backend" in item.nodeid or "cachekitio_metrics" in item.nodeid or "memcached_backend" in item.nodeid: - # Remove the autouse redis isolation fixture for this test - item.fixturenames = [f for f in item.fixturenames if f != "setup_di_for_redis_isolation"] + skip_redis = ( + "file_backend" in item.nodeid + or "cachekitio_metrics" in item.nodeid + or "memcached_backend" in item.nodeid + or "secure_env_fallback" in item.nodeid + ) + if skip_redis: + # Remove autouse redis fixtures for tests that don't need Redis + item.fixturenames = [f for f in item.fixturenames if f not in ("setup_di_for_redis_isolation", "setup_redis_env")] diff --git a/tests/critical/test_secure_env_fallback.py b/tests/critical/test_secure_env_fallback.py new file mode 100644 index 0000000..b6f0ddb --- /dev/null +++ b/tests/critical/test_secure_env_fallback.py @@ -0,0 +1,66 @@ +""" +CRITICAL PATH TEST: cache.secure env var fallback (issue #69) + +Verifies cache.secure resolves CACHEKIT_MASTER_KEY from the environment +when master_key is not passed explicitly. No Redis required. +""" + +from __future__ import annotations + +import pytest + +from cachekit.config.singleton import reset_settings +from cachekit.decorators import cache + +pytestmark = [pytest.mark.critical] + + +class TestSecureEnvFallback: + """Critical: cache.secure must resolve master_key from env var.""" + + def test_secure_reads_master_key_from_env(self, monkeypatch): + """@cache.secure(ttl=300) works when CACHEKIT_MASTER_KEY is set.""" + test_key = "ab" * 32 # 64 hex chars = 32 bytes + monkeypatch.setenv("CACHEKIT_MASTER_KEY", test_key) + reset_settings() + + try: + + @cache.secure(backend=None, ttl=300) + def secure_from_env(x: int) -> int: + return x * 2 + + assert secure_from_env is not None + finally: + reset_settings() + + def test_secure_raises_without_key_or_env(self, monkeypatch): + """@cache.secure raises ValueError when no key available anywhere.""" + monkeypatch.delenv("CACHEKIT_MASTER_KEY", raising=False) + reset_settings() + + try: + with pytest.raises(ValueError, match="CACHEKIT_MASTER_KEY"): + + @cache.secure(backend=None) + def secure_no_key(): + pass + finally: + reset_settings() + + def test_explicit_master_key_takes_precedence(self, monkeypatch): + """Explicit master_key param is used even when env var is set.""" + monkeypatch.setenv("CACHEKIT_MASTER_KEY", "ff" * 32) + reset_settings() + + explicit_key = "aa" * 32 + + try: + + @cache.secure(master_key=explicit_key, backend=None, ttl=60) + def secure_explicit(x: int) -> int: + return x + + assert secure_explicit is not None + finally: + reset_settings()