Modernize keyset loading and test the full support matrix

.github/workflows/ci.yml

@@ -25,8 +25,12 @@ jobs:
             django: "5.2"
           - python: "3.12"
             django: "6.0"
+          - python: "3.13"
+            django: "5.2"
           - python: "3.13"
             django: "6.0"
+          - python: "3.14"
+            django: "5.2"
           - python: "3.14"
             django: "6.0"
 
@@ -53,6 +57,26 @@ jobs:
           PYTHONWARNINGS: default
         run: python -m pytest -c example_project/pytest.ini example_project/example_app/tests
 
+  minimum-tink:
+    name: Python 3.10 / Django 5.2 / Tink 1.13.0
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.10"
+          cache: pip
+          cache-dependency-path: pyproject.toml
+      - name: Install minimum supported Tink
+        run: python -m pip install -e ".[test]" "Django~=5.2.0" "tink==1.13.0"
+      - name: Test minimum supported Tink
+        env:
+          PYTHONWARNINGS: default
+        run: |
+          python -m pytest
+          python -m pytest -c example_project/pytest.ini example_project/example_app/tests
+
   quality:
     runs-on: ubuntu-latest
     timeout-minutes: 10

CHANGELOG.md

@@ -9,10 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Load existing JSON keysets through Tink's explicit `json_proto_keyset_format` APIs, with typed handles and unchanged encrypted-keyset AAD.
+- Exercise all advertised Python/Django combinations and the minimum supported Tink 1.13.0 in CI and tox.
+
 - Share AEAD and deterministic AEAD primitives across fields using the same cached keyset, avoiding repeated wrapper construction while retaining bounded caching and weak manager tracking.
 
 ### Fixed
 
+- Expand user-relative keyset paths before validation and report invalid paths, non-UTF-8 keysets, invalid master primitives, and incompatible AEAD keysets as configuration errors.
+
 - Validate positional database options after Django resolves them. Randomized slug fields now default to `db_index=False`; existing applications should generate and apply the resulting index-removal migration.
 
 - Encrypt binary buffer contents before driver adaptation, fixing PostgreSQL writes that encrypted the string representation of a `psycopg.Binary` adapter. Previously corrupted values require application-specific recovery; this fix does not rewrite stored rows.

README.md

@@ -71,7 +71,7 @@ class Customer(models.Model):
     birth_date = EncryptedDateField(null=True)
 ```
 
-Values are ordinary Python objects on model instances. Django validates them using the corresponding built-in field's validators, encrypts them before database storage, and decrypts them when loading rows.
+Values are ordinary Python objects on model instances. The fields use the corresponding built-in field's validators, encrypt values before database storage, and decrypt them when loading rows. As with ordinary Django models, `save()` does not call `full_clean()` automatically; use a validated model form or call `full_clean()` explicitly when validation is required.
 
 ### Randomized fields
 
@@ -169,6 +169,8 @@ clear_keyset_cache()
 
 Cache invalidation is synchronized with keyset loading and primitive construction. Operations that already obtained a primitive may finish with the old key; subsequent field operations load the replacement. The cache is local to each process, so reload or restart every worker.
 
+For deterministic fields, promoting a new primary key changes the ciphertext used by exact lookups. Retaining old keys permits decryption but does not make new equality queries match rows encrypted under an old primary key, and a unique ciphertext index cannot enforce plaintext uniqueness across key generations. Plan a coordinated data migration before rotating deterministic keys; cache invalidation alone does not solve this.
+
 Changing `keyset=` does not re-encrypt existing rows; it only changes how future reads and writes are processed. Likewise, changing an existing plaintext Django field to an encrypted field requires an explicit staged data migration. Back up data and test recovery before any key or ciphertext migration.
 
 ## Security limitations
@@ -178,7 +180,7 @@ Changing `keyset=` does not re-encrypt existing rows; it only changes how future
 - Encryption does not hide row existence, nullness, ciphertext length, access patterns, or—when deterministic encryption is used—equality patterns.
 - Ordering encrypted columns is permitted by databases but orders ciphertext, not plaintext, and has no useful application meaning.
 - AAD authenticates context but is not secret and is not stored automatically.
-- Validation happens before storage but is not a substitute for authorization, logging controls, backups, or database security.
+- Field validation requires a model form or an explicit `full_clean()` call; encryption is not a substitute for application validation, authorization, logging controls, backups, or database security.
 
 See [SECURITY.md](SECURITY.md) for vulnerability reporting and supported releases.
 

docs/code-review-2026-09.md

@@ -0,0 +1,125 @@
+# Code review and landing plan — 2026-09-05
+
+Reviewed the library, tests, example project, packaging, and CI from `main` at
+`72c1661`. The original suite passed 76 library tests with 95.28% coverage, but
+missed several value-conversion and ORM restriction failures. This stack adds
+behavioral regressions instead of treating coverage percentage as proof of
+correctness.
+
+## Changes in this stack
+
+| Priority | Finding and evidence | Resolution |
+| --- | --- | --- |
+| High | JSON key lookups, date transforms, and late-registered lookups bypassed the ciphertext restrictions. Twelve new cases failed to raise `FieldError`. | [PR #7](https://github.com/script3r/django-tink-fields/pull/7): explicit lookup allowlist and concrete equality classes. |
+| High | With `USE_TZ=True`, SQLite datetime reads lost timezone information. Re-saving under a different default timezone could shift the instant. Six new cases failed. | [PR #8](https://github.com/script3r/django-tink-fields/pull/8): restore the database timezone after decrypting naive datetime representations, preserving serialization. |
+| High | An in-flight `cached_property` getter could publish an old primitive after cache invalidation returned. | [PR #9](https://github.com/script3r/django-tink-fields/pull/9): synchronize construction/publication with invalidation; test concurrent loads and actual rotation for both primitive types. |
+| Medium | One hundred fields sharing a keyset constructed 100 separate primitives. | PR #9: share one primitive per type per cached keyset, retaining bounded caching and weak manager references. |
+| High | PostgreSQL binary writes encrypted the string representation of `psycopg.Binary`, corrupting the original contents. Four real-driver cases failed. | [PR #10](https://github.com/script3r/django-tink-fields/pull/10): convert buffer contents before encryption, and adapt only final ciphertext. |
+| Medium | Positional database options bypassed validation; randomized slug fields silently inherited an index. | [PR #11](https://github.com/script3r/django-tink-fields/pull/11): validate resolved options and serialize `db_index=False` for encrypted slugs. |
+| Medium | User-relative keyset paths were checked before expansion; invalid encodings and incompatible AEAD primitives escaped as low-level exceptions. | Final keyset-loading PR: normalize paths and improve configuration errors. |
+| Medium | CI omitted two advertised Python/Django combinations and never pinned the minimum Tink version. | Final PR: add Python 3.13/3.14 with Django 5.2 and a Tink 1.13.0 job; keep tox aligned. |
+| Low | Keyset loading used the older reader/handle API and untyped handles. | Final PR: use Tink's explicit JSON format API and `KeysetHandle` annotations, with legacy keyset interoperability coverage. |
+
+## Performance evidence
+
+Run `python -m benchmarks.keyset_cache` from the repository root. The script
+reports the median of seven samples, with 20 initializations or 100,000 warm
+encryptions per sample. On this Mac with Python 3.14, at the cache PR boundary:
+
+| Operation | Before | After |
+| --- | ---: | ---: |
+| Initialize 100 managers sharing one keyset | 2.227 ms | 1.466 ms |
+| Warm primitive access and encryption | 0.504 µs | 0.598 µs |
+| Primitive constructions for 100 managers | 100 | 1 |
+
+The improvement is in initialization and wrapper reuse. Locking adds a small
+steady-state cost; these measurements do not establish application throughput
+or performance under contention. Encryption itself occurs outside the cache
+lock. Calls that obtained an old primitive can finish with it, and every worker
+process must reload or restart after rotation.
+
+## Landing and rollout
+
+**Current landing order:** [PR #13](https://github.com/script3r/django-tink-fields/pull/13)
+then [PR #12](https://github.com/script3r/django-tink-fields/pull/12). PR #7 is in
+`main`. PRs #8–#11 were merged into their predecessor branches, so PR #13 carries
+those four original commits into `main`. After #13 lands, change #12's base to
+`main` before merging it (or verify GitHub has retargeted it automatically).
+
+Land the PRs in dependency order. Each PR targets its predecessor so its diff
+contains only that change. After a parent lands, retarget the next PR to `main`
+if GitHub has not done so automatically. Merge commits preserve the stack's
+ancestry; squash or rebase merges require rebasing the remaining branches onto
+the new `main` before landing them.
+
+- Existing correctly serialized ciphertext remains readable. No key or payload
+  format migration is introduced by the lookup, timezone, cache, or loading PRs.
+- For encrypted slug fields, generate and apply the index-removal migration.
+  Positional configurations that violate documented restrictions now fail early.
+- PostgreSQL binary rows already corrupted by adapter stringification require
+  application-specific recovery. In particular, a stored memoryview description
+  does not contain the original bytes and cannot be repaired by this patch.
+- The final README corrects the validation claim: Django model `save()` does not
+  automatically call `full_clean()`.
+
+## Follow-up priorities and limits
+
+1. **High — deterministic rotation needs an explicit migration design.** A new
+   primary key produces different ciphertext. Retaining old enabled keys allows
+   decryption but does not make new exact lookups match old rows. A unique index
+   also cannot enforce plaintext uniqueness across key generations. The README
+   now explains this limitation. A future implementation needs a coordinated
+   rewrite, an explicit key-generation strategy, or a separately designed search
+   index; simply clearing the cache is insufficient.
+
+2. **High — define canonical deterministic representations before expanding
+   backend guarantees.** With Django's PostgreSQL adapter, equal aware datetime
+   instants expressed in Tokyo and UTC produce different encrypted bytes because
+   the serialized strings retain different offsets. This was reproduced without
+   a database server using the real PostgreSQL backend. Changing serialization
+   silently would invalidate equality against existing rows, so it needs a
+   versioned migration plan. UUID representations can also differ by backend.
+
+3. **Medium — add real PostgreSQL and MySQL server CI.** This stack tests SQLite
+   persistence and the real psycopg binary adapter, but does not claim server
+   integration coverage for PostgreSQL, MySQL, or Oracle. Add service-backed
+   tests for schema changes, constraints, datetime conversions, and binary
+   persistence before strengthening the documented backend support.
+
+4. **Medium — remove temporary field-type mutation during validator creation.**
+   `EncryptedField.validators` temporarily changes `_internal_type` on a shared
+   field instance. This is a potential concurrency hazard, not a reproduced
+   failure in this review. A replacement should construct the concrete field's
+   validators without changing shared metadata and retain backend range checks.
+
+5. **Low — improve typing and test organization incrementally.** The older
+   coverage-focused tests contain duplicate assertions and stale line-number
+   comments. Consolidate them around observable behavior while retaining the
+   new regression cases. Package a `py.typed` marker only after testing the
+   public Django field annotations with downstream type checkers. Module splits
+   should preserve public field import paths used in existing migrations.
+
+## Validation
+
+At the top of the stack, all **137 library tests** and **6 example integration
+tests** pass locally on both Python 3.14 / Django 6.0 / Tink 1.16.1 and Python
+3.10 / Django 5.2 / Tink 1.13.0. Library coverage is **97.62%**. Ruff lint/format
+and Pyright pass. Distribution builds and strict Twine validation pass; the
+tested environment has no known vulnerabilities reported by pip-audit, and
+Bandit reports no medium/high findings. GitHub CI provides the remaining interpreter combinations;
+consult each PR's checks for the status of its exact head commit.
+
+The tests cover raw ciphertext/SQL NULL storage, tamper detection, migrations,
+real driver adaptation, timezone round trips, keyset interoperability, real key
+rotation, bounded caching, and concurrent invalidation. They do not establish
+formal cryptographic correctness or recovery of previously corrupted data.
+
+## Upstream references
+
+Tink's [Python keyset example](https://developers.google.com/tink/generate-plaintext-keyset)
+uses `json_proto_keyset_format` with explicit secret-key access. The loading
+change retains the empty associated data used for existing encrypted keysets.
+[Django's custom field documentation](https://docs.djangoproject.com/en/6.0/howto/custom-model-fields/)
+describes the separation between database preparation and conversion on reads;
+its [model validation documentation](https://docs.djangoproject.com/en/6.0/ref/models/instances/#validating-objects)
+explains that `save()` does not call `full_clean()` automatically.

tink_fields/fields.py

@@ -26,7 +26,7 @@
 from django.utils import timezone
 from django.utils.encoding import force_bytes, force_str
 from django.utils.functional import cached_property
-from tink import JsonKeysetReader, TinkError, aead, cleartext_keyset_handle, daead, read_keyset_handle
+from tink import KeysetHandle, TinkError, aead, daead, json_proto_keyset_format, secret_key_access
 
 
 def _register_tink_primitives() -> None:
@@ -110,22 +110,30 @@ def validate(self) -> None:
         if not self.path:
             raise ImproperlyConfigured("Keyset path cannot be None or empty.")
 
-        if not Path(self.path).is_file():
+        try:
+            path = Path(self.path).expanduser().resolve()
+            readable_file = path.is_file()
+        except (OSError, RuntimeError, TypeError, ValueError) as error:
+            raise ImproperlyConfigured(f"Keyset `{self.path}` is not a readable file.") from error
+        if not readable_file:
             raise ImproperlyConfigured(f"Keyset `{self.path}` is not a readable file.")
+        object.__setattr__(self, "path", path)
 
         if not isinstance(self.cleartext, bool):
             raise ImproperlyConfigured("Keyset option `cleartext` must be a boolean.")
 
         if not self.cleartext and self.master_key_aead is None:
             raise ImproperlyConfigured("Encrypted keysets must specify `master_key_aead`.")
+        if not self.cleartext and not isinstance(self.master_key_aead, aead.Aead):
+            raise ImproperlyConfigured("`master_key_aead` must be a Tink Aead primitive.")
 
 
 Primitive = TypeVar("Primitive", aead.Aead, daead.DeterministicAead)
 
 
 @dataclass
 class _KeysetEntry:
-    handle: Any
+    handle: KeysetHandle
     primitives: dict[type[Any], Any] = field(default_factory=dict)
 
 
@@ -201,7 +209,7 @@ def _get_keyset_entry(self) -> _KeysetEntry:
             return self._entry
 
         keyset_config = self._get_keyset_config()
-        keyset_path = Path(keyset_config.path).expanduser().resolve()
+        keyset_path = Path(keyset_config.path)
         try:
             stat = keyset_path.stat()
         except OSError as error:
@@ -224,14 +232,14 @@ def _get_keyset_entry(self) -> _KeysetEntry:
             self._entry = cached_entry
         else:
             try:
-                reader = JsonKeysetReader(keyset_path.read_text(encoding="utf-8"))
+                serialized_keyset = keyset_path.read_text(encoding="utf-8")
                 if keyset_config.cleartext:
-                    handle = cleartext_keyset_handle.read(reader)
+                    handle = json_proto_keyset_format.parse(serialized_keyset, secret_key_access.TOKEN)
                 else:
                     master_key_aead = keyset_config.master_key_aead
                     assert master_key_aead is not None
-                    handle = read_keyset_handle(reader, master_key_aead)
-            except (OSError, TinkError) as error:
+                    handle = json_proto_keyset_format.parse_encrypted(serialized_keyset, master_key_aead, b"")
+            except (OSError, UnicodeError, TinkError) as error:
                 raise ImproperlyConfigured(f"Could not load keyset `{self.keyset_name}`.") from error
 
             self._entry = _KeysetEntry(handle)
@@ -243,7 +251,7 @@ def _get_keyset_entry(self) -> _KeysetEntry:
 
         return self._entry
 
-    def _get_tink_keyset_handle(self) -> Any:
+    def _get_tink_keyset_handle(self) -> KeysetHandle:
         """Return this manager's configured handle."""
         with self._cache_lock:
             return self._get_keyset_entry().handle
@@ -260,7 +268,12 @@ def _get_primitive(self, primitive_class: type[Primitive]) -> Primitive:
     @property
     def aead_primitive(self) -> aead.Aead:
         """Get the AEAD primitive shared by managers using this keyset."""
-        return self._get_primitive(aead.Aead)
+        try:
+            return self._get_primitive(aead.Aead)
+        except TinkError as error:
+            raise ImproperlyConfigured(
+                "Current keyset does not support AEAD. Please use a keyset that contains AEAD keys."
+            ) from error
 
     @property
     def daead_primitive(self) -> daead.DeterministicAead:
@@ -378,7 +391,7 @@ def _get_aad(self) -> bytes:
         return aad
 
     @property
-    def _keyset_handle(self) -> Any:
+    def _keyset_handle(self) -> KeysetHandle:
         """Get the keyset handle for backward compatibility.
 
         Returns: