From 75228724a849048bc8559c89f8485e77f84468a7 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 25 Apr 2026 09:11:29 +1000 Subject: [PATCH 1/5] ci: fix cargo-geiger unsafe check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrong jq paths (.package.functions vs .unsafety.used.functions) produced null, and bc isn't on the runner. Replaced with correct paths and scoped check to cachekit-rs only — checking all deps is meaningless since crypto crates (ring, aes-gcm) are inherently unsafe. --- .github/workflows/security-medium.yml | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/.github/workflows/security-medium.yml b/.github/workflows/security-medium.yml index 94fccd8..0319d89 100644 --- a/.github/workflows/security-medium.yml +++ b/.github/workflows/security-medium.yml @@ -37,29 +37,16 @@ jobs: cargo geiger --output-format Json > geiger-report.json cat geiger-report.json - - name: Check unsafe ratio (< 5%) + - name: Check unsafe in cachekit-rs (must be zero) run: | cd rust - # Parse total and unsafe counts from geiger JSON - TOTAL_FUNCS=$(jq '[.packages[].package.functions.safe + .packages[].package.functions.unsafe] | add' geiger-report.json) - UNSAFE_FUNCS=$(jq '[.packages[].package.functions.unsafe] | add' geiger-report.json) - - if [ "$TOTAL_FUNCS" -eq 0 ]; then - echo "⚠️ No functions found in geiger report" - exit 0 - fi - - UNSAFE_RATIO=$(echo "scale=4; $UNSAFE_FUNCS / $TOTAL_FUNCS * 100" | bc) - - echo "Total functions: $TOTAL_FUNCS" - echo "Unsafe functions: $UNSAFE_FUNCS" - echo "Unsafe ratio: $UNSAFE_RATIO%" - - if (( $(echo "$UNSAFE_RATIO > 5.0" | bc -l) )); then - echo "❌ Unsafe ratio ($UNSAFE_RATIO%) exceeds 5% threshold" + UNSAFE_FUNCS=$(jq '.packages[] | select(.package.id.name == "cachekit-rs") | .unsafety.used.functions.unsafe_' geiger-report.json) + echo "cachekit-rs unsafe functions: $UNSAFE_FUNCS" + if [ "$UNSAFE_FUNCS" -ne 0 ]; then + echo "cachekit-rs has $UNSAFE_FUNCS unsafe functions — must be zero" exit 1 fi - echo "✅ Unsafe ratio ($UNSAFE_RATIO%) is within acceptable limits" + echo "cachekit-rs has zero unsafe functions" - name: Archive geiger report uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7 From d4105bb654a48aad746f2bf4bb7a190909ba8734 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 25 Apr 2026 09:15:03 +1000 Subject: [PATCH 2/5] test: fix flaky file TTL and Python 3.9 asyncio compat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test_exists_expired_ttl: TTL 1s→2s, sleep 1.5s→2.5s (1s expired between set() and exists() under CI load) - test_concurrent_access: move asyncio.Lock() inside async function (Python 3.9 requires a running event loop for Lock constructor) --- tests/integration/test_redis_integration.py | 22 ++++++++++----------- tests/unit/backends/test_file_backend.py | 6 +++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/integration/test_redis_integration.py b/tests/integration/test_redis_integration.py index 30df0ab..c15b63b 100644 --- a/tests/integration/test_redis_integration.py +++ b/tests/integration/test_redis_integration.py @@ -136,19 +136,19 @@ def service_b_func(key): def test_concurrent_access(self): """Concurrent access with distributed locking (async required).""" call_count = 0 - lock = asyncio.Lock() - - @cache(ttl=300) - async def thread_safe_func(key): - nonlocal call_count - async with lock: - call_count += 1 - count = call_count - await asyncio.sleep(0.1) # Simulate work - return f"result_{key}_{count}" async def run_concurrent_requests(): - # Run concurrent requests using asyncio.gather + lock = asyncio.Lock() # Must create inside running loop (Python 3.9 compat) + + @cache(ttl=300) + async def thread_safe_func(key): + nonlocal call_count + async with lock: + call_count += 1 + count = call_count + await asyncio.sleep(0.1) # Simulate work + return f"result_{key}_{count}" + tasks = [thread_safe_func("shared") for _ in range(10)] return await asyncio.gather(*tasks) diff --git a/tests/unit/backends/test_file_backend.py b/tests/unit/backends/test_file_backend.py index 35417ba..b1d4b48 100644 --- a/tests/unit/backends/test_file_backend.py +++ b/tests/unit/backends/test_file_backend.py @@ -936,14 +936,14 @@ def test_exists_expired_ttl_deletes_file(self, backend: FileBackend, config: Fil key = "exists_expired" value = b"value" - # Set with 1 second TTL - backend.set(key, value, ttl=1) + # Set with 2 second TTL (1s too tight under CI load) + backend.set(key, value, ttl=2) # Verify it exists assert backend.exists(key) is True # Wait for expiration - time.sleep(1.5) + time.sleep(2.5) # exists should return False and delete the file result = backend.exists(key) From a032bd99dde6f3e4952a5f2bbe3280c426edfa56 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 25 Apr 2026 09:19:13 +1000 Subject: [PATCH 3/5] ci: guard against empty jq result in geiger check If cachekit-rs isn't found in the geiger report, jq returns empty string and the numeric comparison blows up. Fail cleanly instead. --- .github/workflows/security-medium.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/security-medium.yml b/.github/workflows/security-medium.yml index 0319d89..7687f88 100644 --- a/.github/workflows/security-medium.yml +++ b/.github/workflows/security-medium.yml @@ -41,6 +41,10 @@ jobs: run: | cd rust UNSAFE_FUNCS=$(jq '.packages[] | select(.package.id.name == "cachekit-rs") | .unsafety.used.functions.unsafe_' geiger-report.json) + if [ -z "$UNSAFE_FUNCS" ]; then + echo "cachekit-rs not found in geiger report" + exit 1 + fi echo "cachekit-rs unsafe functions: $UNSAFE_FUNCS" if [ "$UNSAFE_FUNCS" -ne 0 ]; then echo "cachekit-rs has $UNSAFE_FUNCS unsafe functions — must be zero" From d9890f45db5f91bdcb7bcf48d9be65f00c9dc333 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 25 Apr 2026 09:25:58 +1000 Subject: [PATCH 4/5] test: reset settings singleton before cache.secure validation validate_encryption_config() reads CACHEKIT_MASTER_KEY from the pydantic-settings singleton. If a prior test already cached the singleton without the key, setting the env var has no effect. Must reset_settings() after setting env var and in finally cleanup. --- tests/unit/test_l1_only_mode.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/test_l1_only_mode.py b/tests/unit/test_l1_only_mode.py index e681796..ebe646d 100644 --- a/tests/unit/test_l1_only_mode.py +++ b/tests/unit/test_l1_only_mode.py @@ -235,6 +235,10 @@ def production_func() -> str: secure_call_count = 0 old_key = os.environ.get("CACHEKIT_MASTER_KEY") os.environ["CACHEKIT_MASTER_KEY"] = "a" * 64 + # Reset settings singleton so it picks up the env var + from cachekit.config.singleton import reset_settings + + reset_settings() try: @cache.secure(master_key="a" * 64, backend=None) @@ -251,6 +255,7 @@ def secure_func() -> str: os.environ.pop("CACHEKIT_MASTER_KEY", None) else: os.environ["CACHEKIT_MASTER_KEY"] = old_key + reset_settings() # Backend provider should NEVER have been called for any preset mock_provider.return_value.get_backend.assert_not_called() From 42ff95a312481942a26fb85b6241d6571eeb7e9a Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 25 Apr 2026 09:32:47 +1000 Subject: [PATCH 5/5] fix: validate_encryption_config accepts explicit master_key param The validator was checking CACHEKIT_MASTER_KEY env var via the pydantic-settings singleton, ignoring the master_key passed directly to @cache.secure(master_key=...). This caused flaky test failures depending on singleton cache state. Now: explicit master_key param takes precedence over env var. Production warning only fires when key came from env var (not inline). --- src/cachekit/config/validation.py | 45 ++++++++++--------- src/cachekit/decorators/wrapper.py | 2 +- .../unit/test_encryption_config_validation.py | 6 ++- tests/unit/test_l1_only_mode.py | 34 +++++--------- 4 files changed, 40 insertions(+), 47 deletions(-) diff --git a/src/cachekit/config/validation.py b/src/cachekit/config/validation.py index 1963cfd..035613e 100644 --- a/src/cachekit/config/validation.py +++ b/src/cachekit/config/validation.py @@ -30,13 +30,15 @@ class ConfigurationError(Exception): pass -def validate_encryption_config(encryption: bool = False) -> None: +def validate_encryption_config(encryption: bool = False, master_key: str | None = None) -> None: """Validate encryption configuration when encryption is enabled. - Checks that CACHEKIT_MASTER_KEY is set via pydantic-settings when encryption=True. + Checks for a master key: first from the explicit parameter, then from + CACHEKIT_MASTER_KEY env var via pydantic-settings. Args: encryption: Whether encryption is enabled. If False, no validation. + master_key: Explicit master key (hex string). Takes precedence over env var. Raises: ConfigurationError: If encryption config is invalid @@ -59,33 +61,36 @@ def validate_encryption_config(encryption: bool = False) -> None: if not encryption: return - # Get master key from pydantic-settings (handles env vars properly) - from cachekit.config.singleton import get_settings + # Resolve master key: explicit param > env var via settings + resolved_key = master_key + if not resolved_key: + from cachekit.config.singleton import get_settings - settings = get_settings() - master_key = settings.master_key.get_secret_value() if settings.master_key else None + settings = get_settings() + resolved_key = settings.master_key.get_secret_value() if settings.master_key else None - # Check if master_key is set - if not master_key: + if not resolved_key: raise ConfigurationError( - "CACHEKIT_MASTER_KEY environment variable required when encryption=True. " + "Master key required when encryption=True. Either pass master_key= " + "or set CACHEKIT_MASTER_KEY environment variable. " "Generate with: python -c 'import secrets; print(secrets.token_hex(32))'" ) - # Production environment warning - check via settings (already loaded above) - # Check deployment indicators - if not settings.dev_mode: - logger.warning( - "🔒 SECURITY WARNING: Master key loaded from environment variable in PRODUCTION. " - "Environment variables are NOT secure key storage. " - "For production deployments, use secrets management system: " - "HashiCorp Vault, AWS Secrets Manager, Azure Key Vault, or Google Secret Manager. " - "KMS integration planned for future release." - ) + # Production environment warning when key came from env var (not inline) + if not master_key: + from cachekit.config.singleton import get_settings + + settings = get_settings() + if not settings.dev_mode: + logger.warning( + "Master key loaded from environment variable. " + "For production, use a secrets management system " + "(HashiCorp Vault, AWS Secrets Manager, etc.)." + ) # Validate key format and length try: - key_bytes = bytes.fromhex(master_key) + key_bytes = bytes.fromhex(resolved_key) if len(key_bytes) < 32: raise ConfigurationError( f"CACHEKIT_MASTER_KEY must be at least 32 bytes (256 bits). " diff --git a/src/cachekit/decorators/wrapper.py b/src/cachekit/decorators/wrapper.py index b9dd7d8..959a7e4 100644 --- a/src/cachekit/decorators/wrapper.py +++ b/src/cachekit/decorators/wrapper.py @@ -445,7 +445,7 @@ def create_cache_wrapper( # Validate encryption configuration if encryption is enabled from ..config import validate_encryption_config - validate_encryption_config(encryption) + validate_encryption_config(encryption, master_key=master_key) # Note: L1 cache + encryption is supported. # L1 stores encrypted bytes (not plaintext), decryption happens at read time only. diff --git a/tests/unit/test_encryption_config_validation.py b/tests/unit/test_encryption_config_validation.py index bb69d08..34ae70a 100644 --- a/tests/unit/test_encryption_config_validation.py +++ b/tests/unit/test_encryption_config_validation.py @@ -372,8 +372,10 @@ def test_production_warning_logged_for_prod_envs(self, env_name, monkeypatch, ca warning_messages = [record.message for record in caplog.records if record.levelname == "WARNING"] assert len(warning_messages) > 0, f"Expected warning for ENV={env_name}" - # Warning should mention security - has_security_warning = any("SECURITY" in msg.upper() or "WARNING" in msg.upper() for msg in warning_messages) + # Warning should mention env var key usage + has_security_warning = any( + "environment variable" in msg.lower() or "secrets management" in msg.lower() for msg in warning_messages + ) assert has_security_warning def test_no_production_warning_for_dev_env(self, monkeypatch, caplog): diff --git a/tests/unit/test_l1_only_mode.py b/tests/unit/test_l1_only_mode.py index ebe646d..665fb1d 100644 --- a/tests/unit/test_l1_only_mode.py +++ b/tests/unit/test_l1_only_mode.py @@ -14,7 +14,6 @@ from __future__ import annotations -import os import time from unittest.mock import MagicMock, patch @@ -233,29 +232,16 @@ def production_func() -> str: # validate_encryption_config() checks CACHEKIT_MASTER_KEY env var # independently of the inline master_key param, so we must set it. secure_call_count = 0 - old_key = os.environ.get("CACHEKIT_MASTER_KEY") - os.environ["CACHEKIT_MASTER_KEY"] = "a" * 64 - # Reset settings singleton so it picks up the env var - from cachekit.config.singleton import reset_settings - - reset_settings() - try: - - @cache.secure(master_key="a" * 64, backend=None) - def secure_func() -> str: - nonlocal secure_call_count - secure_call_count += 1 - return "secure" - - assert secure_func() == "secure" - assert secure_func() == "secure" - assert secure_call_count == 1, f"@cache.secure L1 miss - called {secure_call_count} times" - finally: - if old_key is None: - os.environ.pop("CACHEKIT_MASTER_KEY", None) - else: - os.environ["CACHEKIT_MASTER_KEY"] = old_key - reset_settings() + + @cache.secure(master_key="a" * 64, backend=None) + def secure_func() -> str: + nonlocal secure_call_count + secure_call_count += 1 + return "secure" + + assert secure_func() == "secure" + assert secure_func() == "secure" + assert secure_call_count == 1, f"@cache.secure L1 miss - called {secure_call_count} times" # Backend provider should NEVER have been called for any preset mock_provider.return_value.get_backend.assert_not_called()