diff --git a/docs/configuration.md b/docs/configuration.md index 3d231534..32b65e07 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -87,7 +87,7 @@ CACHEKIT_MASTER_KEY=<64-hex-char-key; use exactly 32 bytes> # same per-key requirements as CACHEKIT_MASTER_KEY). Entries written under a # listed key stay readable through the rotation window; writes always use # CACHEKIT_MASTER_KEY. More than 3 keys, or the current master key re-appearing -# in this list, is rejected at load (a master_key= argument: when its cache is built). +# in this list, is rejected at load (a master_key= on an encrypting cache: when it is built). CACHEKIT_PREVIOUS_MASTER_KEYS=, # Fail closed on decrypt authentication failures (default: false = fail open/recompute). # When true, AES-GCM auth failures and key-fingerprint mismatches raise diff --git a/docs/features/zero-knowledge-encryption.md b/docs/features/zero-knowledge-encryption.md index 35c3f845..a963b9b6 100644 --- a/docs/features/zero-knowledge-encryption.md +++ b/docs/features/zero-knowledge-encryption.md @@ -406,7 +406,7 @@ export CACHEKIT_MASTER_KEY= export CACHEKIT_PREVIOUS_MASTER_KEYS= ``` -Rules enforced at config load — rejected, never truncated or silently fixed: +Rules enforced at config load or cache build — rejected, never truncated or silently fixed: - **Cap**: at most 3 decrypt-only keys. - **Per-key validation**: identical to `CACHEKIT_MASTER_KEY` (hex-encoded, at least 32 bytes; use exactly 32). @@ -680,10 +680,13 @@ config = EncryptionConfig(enabled=True, master_key=secret_key, raises `KeyringConfigurationError` (a `ValueError` subclass, exported from `cachekit.serializers`) when the decrypt-only keyring is unusable: a previous master key passed directly that is not exactly 32 bytes, more than three previous keys, or the current -key repeated among them. Settings check all three at load, and an encrypting cache checks its -current key against the previous keys when it is built, so behind the decorators this surfaces -only when settings are assigned after that, or on a config-drift read (below). Outside config-drift reads, the fault never -evicts and is not counted on `cachekit_decrypt_failures_total`. Direct `EncryptionWrapper` users +key repeated among them. Settings check all three at load and refuse an assignment that breaks +them, leaving the settings as they were (`previous_master_keys` is a tuple, so a change has to +be an assignment), and an encrypting cache checks its current key against the previous keys when +it is built. Behind the decorators this surfaces on a config-drift read (below), or when a +previous key assigned to the settings repeats the `master_key=` of a cache already built: the +settings never see that key, so its next wrapper build refuses it. Outside config-drift reads, the +fault never evicts and is not counted on `cachekit_decrypt_failures_total`. Direct `EncryptionWrapper` users and callers of the `CacheOperationHandler` read and write methods receive it in both fail modes. Behind the `@cache` decorators, a read of an existing encrypted entry raises it too, from L1 or L2 and from the re-read after a distributed-lock wait, so the function does not run and no @@ -695,6 +698,12 @@ or one of the wrong length, raises `EncryptionError`, and an encryption-disabled claims encryption treats the fault as corruption (miss + evict), because only the unauthenticated header sent it down the decrypt path. +A wrapper keeps the keyring it built. A cache builds one per tenant, on that tenant's first use and +after LRU eviction, so a key change in the settings reaches only wrappers built after it: their +previous keys, and their current key too unless the cache was given `master_key=`. One multi-tenant +cache can therefore run the old keyring for some tenants and the new one for others. To apply a key +change everywhere at once, change the environment and restart the process. + > **⚠️ Key rotation under fail-closed:** with `fail_closed` enabled there is no > silent self-heal — rotating `CACHEKIT_MASTER_KEY` **without retaining the old key > in `CACHEKIT_PREVIOUS_MASTER_KEYS`** makes every pre-rotation entry raise diff --git a/src/cachekit/cache_handler.py b/src/cachekit/cache_handler.py index bc15c398..7fff596a 100644 --- a/src/cachekit/cache_handler.py +++ b/src/cachekit/cache_handler.py @@ -698,8 +698,9 @@ def __init__( None: no prefix, for a handler with no backend behind it. Raises: - ConfigurationError: If encryption config is invalid (missing mode or both modes), or a - master key is present while encryption is None. + ConfigurationError: If encryption config is invalid (missing mode or both modes), a + master key is present while encryption is None, or an encrypting handler's current key + (master_key= or CACHEKIT_MASTER_KEY) is also among the previous master keys. TypeError: If serializer_name is not a string or SerializerProtocol instance, or master_key is bytes (pass ``key.hex()``). @@ -774,11 +775,10 @@ def __init__( encryption = False # The keyring's forward-only rule, refused here at decoration before any backend call, whichever route the - # current key took. Settings run the same check at load, but never see master_key= and take later assignments - # unvalidated; without this, the native keyring refuses only when the first read or write builds it. An - # encryption-disabled handler never encrypts, so it spends no nonce budget: its keyring is built only for a - # config-drift read, whose fault is a miss by design. Nor does it load settings here: a plaintext handler must - # not fail on a malformed keyring setting it never uses. + # current key took. Settings run the same check at load and on assignment, but never see master_key=; without + # this, the native keyring refuses only when the first read or write builds it. An encryption-disabled handler + # never encrypts, so it spends no nonce budget: its keyring is built only for a config-drift read, whose fault + # is a miss by design. This check loads no settings for a plaintext handler. if encryption: settings = get_settings() try: diff --git a/src/cachekit/config/settings.py b/src/cachekit/config/settings.py index 85b68ee1..bc9b17fd 100644 --- a/src/cachekit/config/settings.py +++ b/src/cachekit/config/settings.py @@ -17,17 +17,20 @@ from __future__ import annotations +import functools +import threading from typing import Annotated, Any, Literal, Optional from pydantic import ( + BaseModel, Field, SecretStr, field_validator, model_validator, ) -from pydantic_settings import NoDecode, SettingsConfigDict +from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict -from .validation import RedactingSettings, refuse_current_key_in_previous_keys +from .validation import RedactingSettings, _redacting, refuse_current_key_in_previous_keys # Keyring cap from the protocol spec (spec/encryption.md → "Key Rotation (Keyring)"): # at most 3 decrypt-only previous keys. Exceeding the cap is a configuration error, @@ -35,6 +38,13 @@ # MAX_DECRYPT_ONLY_KEYS, which re-validates behind the FFI boundary. MAX_PREVIOUS_MASTER_KEYS = 3 +# Serializes CachekitConfig assignments: each validates the whole state it would leave, so two +# assignments that are each valid alone (a new master_key, and that key added to previous_master_keys) +# cannot land together unchecked. An assignment commits by swapping in its copy's whole state, so every +# other change (a private attribute, a delete) takes the lock too, or the swap would undo it. Reentrant: +# a subclass's validator or property setter may assign a field or a private attribute while it is held. +_ASSIGNMENT_LOCK = threading.RLock() + class CachekitConfig(RedactingSettings): """Backend-agnostic cache configuration. @@ -122,6 +132,8 @@ class CachekitConfig(RedactingSettings): # logs. errors()/json() ignore this flag; RedactingSettings sanitizes # those surfaces. hide_input_in_errors=True, + # Assignment after load runs every field and model validator; __setattr__ makes a refusal write nothing. + validate_assignment=True, ) # Generic cache configuration (backend-agnostic) @@ -205,8 +217,9 @@ class CachekitConfig(RedactingSettings): default=None, description="Master encryption key (hex-encoded; use exactly 32 bytes, 64 hex characters)", ) - previous_master_keys: Annotated[list[SecretStr], NoDecode] = Field( - default_factory=list, + # A tuple, so the keyring cannot be edited in place, where no validator runs: a change is an assignment. + previous_master_keys: Annotated[tuple[SecretStr, ...], NoDecode] = Field( + default_factory=tuple, description=( "Decrypt-only previous master keys for key rotation (env: " "CACHEKIT_PREVIOUS_MASTER_KEYS, comma-separated hex). Entries written " @@ -277,6 +290,46 @@ def validate_previous_master_keys(self) -> CachekitConfig: return self + def __setattr__(self, name: str, value: Any) -> None: + """Validate an assignment on a copy first, and write it only if the copy passes. + + validate_assignment alone writes the new value before the model validator runs, so a refused keyring + assignment would stay on the instance, readable by other threads until the error surfaces. The value + is dropped in a finally: the raised error's traceback holds this frame (CWE-532). + """ + # A private attribute: nothing to validate, so no copy. It still takes the lock, so it cannot land between + # another thread's model_copy() and swap and be lost. Any other name goes through the guard, so a mistyped + # field's value is refused inside _redacting too. + if name in type(self).__private_attributes__: + with _ASSIGNMENT_LOCK: + super().__setattr__(name, value) + return + candidate = None + try: + with _ASSIGNMENT_LOCK: + # A subclass property: run its setter once, on this instance. Each field it assigns comes back + # through this guard; replaying it on a copy would apply a non-idempotent setter twice. Redacted like + # the copy path, so a refused key leaves no pydantic frame holding it. + if isinstance(getattr(type(self), name, None), property): + _redacting(functools.partial(BaseSettings.__setattr__, self, name, value), type(self).__name__) + return + candidate = self.model_copy() + # BaseSettings.__setattr__, not this override: pydantic's validated assignment, on the copy. + _redacting(functools.partial(BaseSettings.__setattr__, candidate, name, value), type(self).__name__) + # Commit the state the copy validated, with no second validation: `value` may be a spent generator, + # and a non-idempotent validator would change it again. Every slot pydantic keeps instance state in, as + # model_copy() fills them: __dict__, the fields set, an extra="allow" subclass's undeclared names, and + # the private state a subclass's model validator may have derived on the copy. + for slot in BaseModel.__slots__: + object.__setattr__(self, slot, getattr(candidate, slot)) + finally: + del value, candidate # the copy holds the refused value + + def __delattr__(self, name: str) -> None: + """Delete under the assignment lock, so an assignment's swap cannot undo the delete.""" + with _ASSIGNMENT_LOCK: + super().__delattr__(name) + def __repr__(self) -> str: """Return string representation with sensitive information masked. diff --git a/src/cachekit/serializers/encryption_wrapper.py b/src/cachekit/serializers/encryption_wrapper.py index a997320e..4e795bf2 100644 --- a/src/cachekit/serializers/encryption_wrapper.py +++ b/src/cachekit/serializers/encryption_wrapper.py @@ -311,10 +311,11 @@ def _setup_encryption(self, master_key: SecretBytes | None, previous_master_keys # Decrypt-only previous keys from settings if not provided (key rotation, # spec/encryption.md → "Key Rotation (Keyring)"). Settings enforce the cap # of 3, per-key hex/length validation, and the forward-only subset check - # at load, and CacheSerializationHandler repeats the subset check for an - # encrypting cache's master_key= when it is built; the Rust Keyring - # re-validates all three behind the FFI boundary for wrappers constructed - # with explicit parameters and for settings assigned after those checks. + # at load and on every assignment, and CacheSerializationHandler repeats the + # subset check for an encrypting cache's master_key= when it is built; the + # Rust Keyring re-validates all three behind the FFI boundary. This wrapper + # keeps the keyring it builds: a later change to the settings reaches only + # wrappers built after it. # Keyring config errors below raise KeyringConfigurationError, NEVER # EncryptionError: EncryptionError is a SerializationError, which the # read-path policy (handle_decrypt_failure) classifies as corruption → @@ -331,8 +332,8 @@ def _setup_encryption(self, master_key: SecretBytes | None, previous_master_keys settings = get_settings() previous_master_keys = [SecretBytes(bytes.fromhex(key.get_secret_value())) for key in settings.previous_master_keys] - # The hex rule again, for keys read from settings: settings validate them at load, but the settings - # object takes later assignments unvalidated, and the Rust Keyring below checks only 16 bytes. + # The hex rule again, for keys read from settings: settings validate them at load and on assignment, and + # this stays as defence in depth because the Rust Keyring below checks only 16 bytes. for position, previous_key in enumerate(previous_master_keys): if len(previous_key) < 32: raise KeyringConfigurationError( diff --git a/tests/unit/config/test_redacting_settings.py b/tests/unit/config/test_redacting_settings.py index 2f3d9bd7..b6f2a484 100644 --- a/tests/unit/config/test_redacting_settings.py +++ b/tests/unit/config/test_redacting_settings.py @@ -42,6 +42,16 @@ from cachekit.serializers.encryption_wrapper import EncryptionError, EncryptionWrapper _KEY_HEX = "ab" * 32 +# No two 16-character windows of it are alike, so a truncated rendering of it is still found. +_DISTINCT_KEY_HEX = bytes(range(32)).hex() + +# Post-load assignments to the settings that are refused: by a field, by the model validator, and to a name +# that is no field. +_SETTINGS_ASSIGNMENT_REFUSALS: dict[str, tuple[str, object]] = { + "field-level": ("previous_master_keys", {_DISTINCT_KEY_HEX: 1}), + "model-level": ("previous_master_keys", [_DISTINCT_KEY_HEX, "01" * 32, "02" * 32, "03" * 32]), + "mistyped-field": ("previous_master_key", _DISTINCT_KEY_HEX), +} BACKEND_CONFIGS: list[type[BaseBackendConfig]] = [ RedisBackendConfig, @@ -633,6 +643,51 @@ def _cachekit_locals_holding(exc: BaseException, secret: str | bytes, *, below_c return found +@pytest.mark.unit +class TestSettingsAssignmentRedaction: + """A refused assignment to the loaded settings carries no route to the key it was given, whole or truncated.""" + + @pytest.mark.parametrize( + ("field", "value"), _SETTINGS_ASSIGNMENT_REFUSALS.values(), ids=_SETTINGS_ASSIGNMENT_REFUSALS.keys() + ) + def test_refused_assignment_redacts_the_key(self, field: str, value: object) -> None: + with pytest.raises(ValidationError) as exc_info: + setattr(singleton.get_settings(), field, value) + + _assert_no_route_to(exc_info.value, _DISTINCT_KEY_HEX) + rendered = str(exc_info.value) + repr(exc_info.value.errors()) + exc_info.value.json() + fragments = {_DISTINCT_KEY_HEX[i : i + 16] for i in range(len(_DISTINCT_KEY_HEX) - 15)} + assert [fragment for fragment in fragments if fragment in rendered] == [] + + @pytest.mark.parametrize( + ("field", "value"), _SETTINGS_ASSIGNMENT_REFUSALS.values(), ids=_SETTINGS_ASSIGNMENT_REFUSALS.keys() + ) + def test_refused_assignment_leaves_no_frame_local(self, field: str, value: object) -> None: + """Pydantic's own assignment frames hold the raw value too, so the guard must re-raise from above them: the + cachekit-only walk of TestEntryPointFrameLocals would pass without that.""" + with pytest.raises(ValidationError) as exc_info: + setattr(singleton.get_settings(), field, value) + + assert _cachekit_locals_holding(exc_info.value, _DISTINCT_KEY_HEX, below_caller=True) == [] + + def test_refused_assignment_through_a_property_leaves_no_frame_local(self) -> None: + """A subclass property setter that assigns a key runs outside the copy, so it needs the same boundary.""" + + class _Rotating(CachekitConfig): + def _rotate(self, key: str) -> None: + self.master_key = key # type: ignore[assignment] + + rotate = property(fset=_rotate) + + config = _Rotating(previous_master_keys=(_DISTINCT_KEY_HEX,)) # type: ignore[arg-type] + with pytest.raises(ValidationError) as exc_info: + config.rotate = _DISTINCT_KEY_HEX + + assert config.master_key is None + _assert_no_route_to(exc_info.value, _DISTINCT_KEY_HEX) + assert _cachekit_locals_holding(exc_info.value, _DISTINCT_KEY_HEX, below_caller=True) == [] + + @pytest.mark.unit class TestRedactingSettingsFrameLocals: """No cachekit frame on a raised config error's traceback keeps the raw input (CWE-532).""" @@ -1073,6 +1128,15 @@ def _api_key_rows(form: Callable[[str], Any]) -> dict[str, _EntryPointRow]: ConfigurationError, _SHORT_KEY_HEX, ), + **{ + f"settings-assignment-{name}": ( + {}, + lambda field=field, value=value: setattr(singleton.get_settings(), field, value), + ValidationError, + _DISTINCT_KEY_HEX, + ) + for name, (field, value) in _SETTINGS_ASSIGNMENT_REFUSALS.items() + }, } diff --git a/tests/unit/protocol/test_encryption_master_key_vectors.py b/tests/unit/protocol/test_encryption_master_key_vectors.py index 833ccf2b..e8a50da6 100644 --- a/tests/unit/protocol/test_encryption_master_key_vectors.py +++ b/tests/unit/protocol/test_encryption_master_key_vectors.py @@ -75,7 +75,8 @@ # One refusal each: a non-hex string, or one that decodes short. Anything else (a missing key) fails the match. DECORATOR_REFUSAL = r"CACHEKIT_MASTER_KEY must be (hex-encoded|at least 32 bytes)" WRAPPER_REFUSAL = r"Invalid master key format|Master key must be at least 32 bytes" -# The refusal each keyring reject row gets at decoration, from the settings validator, on both current-key routes. +# The refusal each keyring reject row gets at decoration from the settings validator; _keyring_refusal names the one +# exception, a repeat row on master_key_argument. _SETTINGS_REFUSAL = r"(?s)^1 validation error for CachekitConfig\n.*Value error, " _REPEAT_REFUSAL = "master_key must not appear in previous_master_keys" KEYRING_REFUSALS = { @@ -88,7 +89,7 @@ def _keyring_refusal(name: str, route: str) -> tuple[type[Exception], str]: - """The settings never see a master_key=, so the decorator's handler refuses its repeat, as it does a bad key.""" + """The settings never see a master_key=, so the decorator's handler refuses its repeat when it is built.""" if name in REPEAT_ROWS and route == "master_key_argument": return ConfigurationError, f"^{_REPEAT_REFUSAL}: " return ValidationError, KEYRING_REFUSALS[name] diff --git a/tests/unit/test_key_rotation_keyring.py b/tests/unit/test_key_rotation_keyring.py index 2c89393d..47029bd7 100644 --- a/tests/unit/test_key_rotation_keyring.py +++ b/tests/unit/test_key_rotation_keyring.py @@ -76,9 +76,9 @@ def test_env_blank_segments_ignored(self, monkeypatch): monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", f" {K1.hex()} ,, ") assert len(CachekitConfig().previous_master_keys) == 1 - def test_default_is_empty_list(self, monkeypatch): + def test_default_is_empty(self, monkeypatch): monkeypatch.delenv("CACHEKIT_PREVIOUS_MASTER_KEYS", raising=False) - assert CachekitConfig().previous_master_keys == [] + assert CachekitConfig().previous_master_keys == () def test_more_than_three_keys_raises_never_truncates(self): four = [SecretStr(f"{i:02x}" * 32) for i in range(1, 5)] @@ -152,6 +152,287 @@ def test_validation_errors_never_echo_key_material(self, monkeypatch): monkeypatch.delenv(key) +_REFUSED_ASSIGNMENTS = { + "previous-is-current": ("previous_master_keys", [SecretStr(K2.hex())], "must not appear in previous_master_keys"), + "previous-is-current-uppercase": ( + "previous_master_keys", + [SecretStr(K2.hex().upper())], + "must not appear in previous_master_keys", + ), + "current-is-previous": ("master_key", SecretStr(K1.hex()), "must not appear in previous_master_keys"), + "fourth-previous": ( + "previous_master_keys", + [SecretStr(f"{i:02x}" * 32) for i in range(4, 8)], + f"at most {MAX_PREVIOUS_MASTER_KEYS}", + ), + "previous-not-hex": ("previous_master_keys", [SecretStr("zz" * 32)], "not valid hex"), + "previous-short": ("previous_master_keys", [SecretStr("aa" * 31)], "at least 32 bytes"), +} + + +class TestSettingsAssignment: + """An assignment to the loaded settings runs the load-time rules, and a refused one writes nothing.""" + + @pytest.fixture + def settings(self, monkeypatch) -> CachekitConfig: + monkeypatch.setenv("CACHEKIT_MASTER_KEY", K2.hex()) + monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", K1.hex()) + reset_settings() + return get_settings() + + @pytest.mark.parametrize(("field", "value", "reason"), _REFUSED_ASSIGNMENTS.values(), ids=_REFUSED_ASSIGNMENTS.keys()) + def test_refused_assignment_leaves_settings_as_they_were(self, settings, field, value, reason): + before = getattr(settings, field) + + with pytest.raises(ValidationError, match=reason): + setattr(settings, field, value) + + assert getattr(settings, field) is before + + @pytest.mark.parametrize("case", ["previous-is-current", "previous-is-current-uppercase", "current-is-previous"]) + def test_refused_value_is_never_written(self, settings, monkeypatch, case): + """The keyring rules run in the model validator, which pydantic's validate_assignment calls only after it has + written the new value. The repeat check is the last of them, so a spy there sees what a concurrent reader of + the live settings would see while they run: the old values.""" + import cachekit.config.settings as settings_module + + field, value, reason = _REFUSED_ASSIGNMENTS[case] + before = (settings.master_key, settings.previous_master_keys) + seen: list[tuple[Any, Any]] = [] + refuse = settings_module.refuse_current_key_in_previous_keys + + def spy(*args: Any) -> None: + seen.append((settings.master_key, settings.previous_master_keys)) + refuse(*args) + + monkeypatch.setattr(settings_module, "refuse_current_key_in_previous_keys", spy) + + with pytest.raises(ValidationError, match=reason): + setattr(settings, field, value) + + assert len(seen) == 1 + assert seen[0][0] is before[0] + assert seen[0][1] is before[1] + + def test_previous_keys_cannot_be_edited_in_place(self, settings): + """No validator runs on an in-place edit, so the keyring takes none: a change has to be an assignment.""" + with pytest.raises(AttributeError): + settings.previous_master_keys.append(SecretStr(K2.hex().upper())) # type: ignore[attr-defined] + + assert settings.previous_master_keys == (SecretStr(K1.hex()),) + + @pytest.mark.parametrize( + "make", + [ + pytest.param(lambda keys: (key for key in keys), id="generator"), + pytest.param(lambda keys: map(str, keys), id="map"), + ], + ) + def test_one_shot_iterable_lands_whole(self, settings, make): + """The guard validates the value on a copy, so the live settings must store what the copy validated: a second + pass over a one-shot iterable would find it empty and drop every previous key without an error.""" + settings.previous_master_keys = make([K1.hex(), K3.hex()]) + + assert settings.previous_master_keys == (SecretStr(K1.hex()), SecretStr(K3.hex())) + + def test_iterable_that_changes_between_passes_cannot_land_unchecked(self, settings): + """An iterable that yields a valid keyring once and the current key after must not reach the live settings.""" + passes = iter([[K3.hex()], [K2.hex()]]) + + class _Shifting: + def __iter__(self): + return iter(next(passes)) + + settings.previous_master_keys = _Shifting() # type: ignore[assignment] + + assert settings.previous_master_keys == (SecretStr(K3.hex()),) + + def test_validator_that_sets_a_private_attribute_does_not_deadlock(self, monkeypatch): + """A subclass's validator may set a private attribute while the guard holds its lock. A fresh lock of the guard's kind keeps a + regression from blocking every later assignment in this process.""" + import threading + + from pydantic import PrivateAttr, model_validator + + import cachekit.config.settings as settings_module + + monkeypatch.setattr(settings_module, "_ASSIGNMENT_LOCK", type(settings_module._ASSIGNMENT_LOCK)()) # same kind, fresh + + class _Marking(CachekitConfig): + _validated: bool = PrivateAttr(default=False) + + @model_validator(mode="after") + def _mark(self) -> _Marking: + self._validated = True + return self + + config = _Marking() + assigning = threading.Thread(target=setattr, args=(config, "l1_max_size_mb", 7), daemon=True) + assigning.start() + assigning.join(timeout=5) + + assert not assigning.is_alive() + assert config.l1_max_size_mb == 7 + + def test_property_setter_that_assigns_a_field_does_not_deadlock(self, monkeypatch): + """A subclass property setter that assigns a field runs on the copy while the guard holds its lock, then + assigns again through the guard. A fresh lock of the guard's kind keeps a regression from blocking later + assignments.""" + import threading + + import cachekit.config.settings as settings_module + + monkeypatch.setattr(settings_module, "_ASSIGNMENT_LOCK", type(settings_module._ASSIGNMENT_LOCK)()) # same kind, fresh + + class _Sized(CachekitConfig): + @property + def size(self) -> int: + return self.l1_max_size_mb + + @size.setter + def size(self, value: int) -> None: + self.l1_max_size_mb = value + + config = _Sized() + assigning = threading.Thread(target=setattr, args=(config, "size", 5), daemon=True) + assigning.start() + assigning.join(timeout=5) + + assert not assigning.is_alive() + assert config.l1_max_size_mb == 5 + + @pytest.mark.parametrize( + ("change", "landed"), + [ + pytest.param(lambda config: setattr(config, "_note", "set"), lambda config: config._note == "set", id="set"), + pytest.param(lambda config: delattr(config, "_note"), lambda config: not hasattr(config, "_note"), id="delete"), + ], + ) + def test_private_change_from_another_thread_is_not_undone(self, monkeypatch, change, landed): + """An assignment commits by swapping in its copy's private state, so a private attribute another thread sets or + deletes between the copy and the commit must wait for the lock instead of being undone by the swap.""" + import threading + + from pydantic import PrivateAttr, model_validator + + import cachekit.config.settings as settings_module + + lock = type(settings_module._ASSIGNMENT_LOCK)() # same kind, fresh + progressed = threading.Event() # the other thread reached the lock, or made its change without it + + class _Reporting: + """The guard's lock, reporting when the other thread reaches it.""" + + def __enter__(self) -> None: + if threading.current_thread() is changing: + progressed.set() + lock.acquire() + + def __exit__(self, *exc_info: object) -> None: + lock.release() + + monkeypatch.setattr(settings_module, "_ASSIGNMENT_LOCK", _Reporting()) + + class _Noted(CachekitConfig): + _note: str = PrivateAttr(default="") + + @model_validator(mode="after") + def _change_meanwhile(self) -> _Noted: + if armed: # on the copy, between model_copy() and the commit + changing.start() + progressed.wait(timeout=5) + return self + + def change_live() -> None: + change(config) + progressed.set() + + changing = threading.Thread(target=change_live, daemon=True) + armed = False + config = _Noted() + armed = True + config.l1_max_size_mb = 7 + changing.join(timeout=5) + + assert not changing.is_alive() + assert config.l1_max_size_mb == 7 + assert landed(config) + + def test_property_setter_runs_once_on_the_live_settings(self): + """A setter that is not idempotent must apply once, a write-only property must still be assignable, and what the + setter writes to a private attribute must land on the live settings, not on a discarded copy.""" + from pydantic import PrivateAttr + + class _Growing(CachekitConfig): + _grown_by: int = PrivateAttr(default=0) + + def _grow(self, by: int) -> None: + self._grown_by += by + self.l1_max_size_mb += by + + grow = property(fset=_grow) + + config = _Growing(l1_max_size_mb=10) + config.grow = 5 + + assert config.l1_max_size_mb == 15 + assert config._grown_by == 5 + + def test_assigned_value_is_validated_once(self): + """The live settings take the state the copy validated, so a validator that is not idempotent runs once.""" + from pydantic import field_validator + + class _Bumping(CachekitConfig): + @field_validator("l1_max_size_mb") + @classmethod + def _bump(cls, value: int) -> int: + return value + 1 + + config = _Bumping() + config.l1_max_size_mb = 5 + + assert config.l1_max_size_mb == 6 + assert "l1_max_size_mb" in config.model_fields_set + + def test_private_state_a_model_validator_derives_lands(self): + """Model validators run on the copy, so what one writes to a private attribute must be committed with it.""" + from pydantic import PrivateAttr, model_validator + + class _Derived(CachekitConfig): + _l1_bytes: int = PrivateAttr(default=0) + + @model_validator(mode="after") + def _derive(self) -> _Derived: + self._l1_bytes = self.l1_max_size_mb * 1024 * 1024 + return self + + config = _Derived() + config.l1_max_size_mb = 7 + + assert config._l1_bytes == 7 * 1024 * 1024 + + def test_extra_attribute_on_an_extra_allow_subclass_lands(self): + """Pydantic stores an undeclared name in __pydantic_extra__, not __dict__, so the commit must carry it.""" + from pydantic_settings import SettingsConfigDict + + class _Open(CachekitConfig): + model_config = SettingsConfigDict(extra="allow") + + config = _Open() + config.custom = 1 + + assert config.custom == 1 + assert config.model_extra == {"custom": 1} + + def test_valid_assignment_lands_validated(self, settings): + settings.previous_master_keys = [K3.hex()] # type: ignore[list-item] + settings.master_key = K1.hex() # type: ignore[assignment] + + assert settings.previous_master_keys == (SecretStr(K3.hex()),) + assert isinstance(settings.master_key, SecretStr) + assert settings.master_key.get_secret_value() == K1.hex() + + class _CallRecordingBackend: """Backend that records every call, to prove a refusal came before any.""" @@ -220,7 +501,8 @@ def test_refused_at_decoration(self, monkeypatch, decorate, previous): assert backend.calls == [] def test_reads_previous_keys_as_they_stand_at_decoration(self, monkeypatch): - """Settings validate no assignment, so a check made only at load would miss this one.""" + """Settings hold no current key here, so they accept this assignment; only a check made when the cache is + built sees it repeat the master_key= argument.""" backend = _CallRecordingBackend() cache.secure(backend=backend, master_key=KA.hex())(lambda: None) # accepted: no previous keys yet @@ -229,18 +511,22 @@ def test_reads_previous_keys_as_they_stand_at_decoration(self, monkeypatch): cache.secure(backend=backend, master_key=KA.hex())(lambda: None) assert backend.calls == [] - def test_reads_an_assigned_settings_master_key(self, monkeypatch): - """The same holds for the current key itself, when it comes from settings assigned after load.""" - monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", KA.hex()) - reset_settings() - monkeypatch.setattr(get_settings(), "master_key", SecretStr(KA.hex())) + def test_reads_the_settings_master_key_as_it_stands_at_build(self, monkeypatch): + """The same holds for the current key itself, when it comes from settings. Settings refuse the repeat at load + and on assignment, so unvalidated settings stand in for it: the handler's check stays as defence in depth.""" + import cachekit.cache_handler as cache_handler + + unvalidated = CachekitConfig.model_construct( + master_key=SecretStr(KA.hex()), previous_master_keys=[SecretStr(KA.hex())], encryption_fail_closed=False + ) + monkeypatch.setattr(cache_handler, "get_settings", lambda: unvalidated) with pytest.raises(ConfigurationError, match=REPEAT_REFUSAL): CacheSerializationHandler(encryption=True, single_tenant_mode=True) - def test_plaintext_handler_reads_no_settings(self, monkeypatch): - """A handler that neither encrypts nor defers fail-closed to settings never loads them, so a malformed - keyring setting cannot stop it being built.""" + def test_plaintext_handler_construction_reads_no_settings(self, monkeypatch): + """Building a handler that neither encrypts nor defers fail-closed to settings never loads them, so a malformed + keyring setting cannot stop it being built. Its later operations may still read settings.""" monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", "not-hex") reset_settings() @@ -575,8 +861,8 @@ def encryption_fingerprints(self, tenant_id: str) -> list[bytes]: def _settings_previous_keys(monkeypatch: pytest.MonkeyPatch, keys: list[bytes]) -> None: - """Hand the wrapper previous keys that skip settings-load validation, as a settings - object assigned to after load does (it validates no assignment). A key passed + """Hand the wrapper previous keys that skip settings validation, which settings refuse at + load and on assignment: the wrapper's own check is defence in depth. A key passed explicitly never gets this far: __init__ holds it to exactly 32 bytes.""" import cachekit.serializers.encryption_wrapper as ew