-
Notifications
You must be signed in to change notification settings - Fork 0
feat: support explicit per-function encryption opt-out (tri-state) #151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,20 +290,35 @@ class EncryptionConfig: | |
| single_tenant_mode automatically; if using EncryptionConfig directly (e.g. with | ||
| @cache.io), you must set it explicitly. | ||
|
|
||
| Tri-state ``enabled`` (issue #128): a plain bool cannot tell "user left it unset" | ||
| from "user explicitly disabled", so a deliberate opt-out was silently overridden by | ||
| fleet-wide CACHEKIT_MASTER_KEY auto-detection. ``enabled`` is therefore None/True/False: | ||
| - None (default): unset — defer to CACHEKIT_MASTER_KEY auto-detection downstream. | ||
| - True: force client-side encryption ON (requires master_key + tenant mode). | ||
| - False: explicit hard opt-out — never encrypt, even when a master key is present. | ||
|
|
||
| Attributes: | ||
| enabled: Enable client-side encryption (default: False) | ||
| master_key: Hex-encoded master key for key derivation (required if enabled) | ||
| enabled: Tri-state encryption flag (default: None = unset/auto-detect). | ||
| True = force-on, False = explicit opt-out. | ||
| master_key: Hex-encoded master key for key derivation (required if enabled=True) | ||
| tenant_extractor: Optional callable for per-tenant key derivation (default: None) | ||
| single_tenant_mode: Explicitly enable single-tenant mode (default: False) | ||
| deployment_uuid: Optional deployment-specific UUID for single-tenant mode (default: None) | ||
|
|
||
| Examples: | ||
| Disabled by default (no encryption): | ||
| Unset by default (defers to auto-detection, no encryption forced): | ||
|
|
||
| >>> config = EncryptionConfig() | ||
| >>> config.enabled | ||
| >>> config.enabled is None | ||
| True | ||
| >>> config.validate() # No error when unset | ||
|
|
||
| Explicit opt-out (never encrypts, even with CACHEKIT_MASTER_KEY set): | ||
|
|
||
| >>> off = EncryptionConfig(enabled=False) | ||
| >>> off.enabled | ||
| False | ||
| >>> config.validate() # No error when disabled | ||
| >>> off.validate() # No error when explicitly disabled | ||
|
|
||
| Single-tenant encryption: | ||
|
|
||
|
|
@@ -326,7 +341,7 @@ class EncryptionConfig: | |
| cachekit.config.validation.ConfigurationError: Encryption requires explicit tenant mode... | ||
| """ | ||
|
|
||
| enabled: bool = False | ||
| enabled: bool | None = None | ||
| master_key: str | None = field(default=None, repr=False) | ||
| tenant_extractor: Callable[..., str] | None = None | ||
| single_tenant_mode: bool = False | ||
|
|
@@ -335,10 +350,18 @@ class EncryptionConfig: | |
| def validate(self) -> None: | ||
| """Validate encryption configuration. | ||
|
|
||
| Only the explicit force-on state (enabled=True) requires a master key. The | ||
| unset (None) and explicit opt-out (False) states are both falsy and skip | ||
| validation — None defers to downstream auto-detection, False never encrypts. | ||
|
|
||
| The master key may be supplied inline or via the CACHEKIT_MASTER_KEY env var | ||
| (resolved here so force-on works fleet-wide without inlining the key, matching | ||
| the handler's own resolution). | ||
|
|
||
| Raises: | ||
| ConfigurationError: If encryption enabled but master_key not set | ||
| ConfigurationError: If encryption enabled but no master_key (inline or env) | ||
| """ | ||
| if self.enabled and not self.master_key: | ||
| if self.enabled and not self._resolve_master_key(): | ||
| raise ConfigurationError( | ||
| "encryption.enabled=True requires encryption.master_key. " | ||
| "Set CACHEKIT_MASTER_KEY environment variable or pass master_key parameter." | ||
|
|
@@ -357,3 +380,17 @@ def validate(self) -> None: | |
| "Cannot use both tenant_extractor and single_tenant_mode. " | ||
| "Choose multi-tenant (tenant_extractor) OR single-tenant (single_tenant_mode=True)." | ||
| ) | ||
|
|
||
| def _resolve_master_key(self) -> str | None: | ||
| """Resolve the master key from the inline value, falling back to env settings. | ||
|
|
||
| Inline master_key takes precedence; otherwise read CACHEKIT_MASTER_KEY via the | ||
| settings singleton. Mirrors validate_encryption_config so the config-level and | ||
| handler-level views of "is a key available?" stay consistent. | ||
| """ | ||
| if self.master_key: | ||
| return self.master_key | ||
| from cachekit.config.singleton import get_settings | ||
|
|
||
| settings = get_settings() | ||
| return settings.master_key.get_secret_value() if settings.master_key else None | ||
|
Comment on lines
+384
to
+396
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Audit master-key typing and unwrapping sites before changing the surface to SecretStr.
rg -n -C2 '\bmaster_key\b|SecretStr|get_secret_value\(' src/cachekit testsRepository: cachekit-io/cachekit-py Length of output: 50380 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Show EncryptionConfig type definitions + _resolve_master_key usage
sed -n '320,430p' src/cachekit/config/nested.py | cat -n
# Show validate_encryption_config master-key resolution
sed -n '1,140p' src/cachekit/config/validation.py | cat -n
# Show cache_handler master_key typing/autodetect and where it unwraps settings.master_key
sed -n '240,620p' src/cachekit/cache_handler.py | cat -nRepository: cachekit-io/cachekit-py Length of output: 30483 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "\. _resolve_master_key\(|_resolve_master_key\(" src/cachekit/config/nested.py src/cachekit | head -n 200Repository: cachekit-io/cachekit-py Length of output: 421 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Show CachekitConfig.master_key definition + type
rg -n "class CachekitConfig|master_key: Optional\\[SecretStr\\]" -n src/cachekit/config/settings.py
sed -n '200,260p' src/cachekit/config/settings.py | cat -n
# Also inspect get_settings/singleton for master_key typing
rg -n "def get_settings|master_key" src/cachekit/config/singleton.py src/cachekit/config/__init__.py 2>/dev/null || true
ls src/cachekit/config | headRepository: cachekit-io/cachekit-py Length of output: 2719 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect singleton typing for master_key
sed -n '1,120p' src/cachekit/config/singleton.py | cat -n
# Inspect EncryptionConfig definition in nested.py (imports + field types)
sed -n '1,120p' src/cachekit/config/nested.py | cat -n
sed -n '240,430p' src/cachekit/config/nested.py | cat -nRepository: cachekit-io/cachekit-py Length of output: 15812 Avoid unwrapping
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve
CACHEKIT_MASTER_KEYfor the forced-on path as well.This branch only materialises
master_keyfrom settings whenencryption is None. In the explicitencryption=Truepath,EncryptionConfig.validate()can succeed on an env key, butcreate_cache_wrapper()still forwardsconfig.encryption.master_key=None, soself.master_keystays unset and_get_cached_encryption_wrapper()later builds theEncryptionWrapperwithout a key. That breaks the advertisedencryption=True+ env-only behaviour.🤖 Prompt for AI Agents