feat(rotation): add zero-downtime key rotation utility#147
Merged
Alberto-Codes merged 6 commits intoMar 28, 2026
Conversation
Adds `rotate_encryption_keys()` — a standalone async function that re-encrypts all session data across all four tables from one backend to another. Designed for same-backend passphrase rotation where `additional_backends` cannot be used (duplicate `backend_id`). - Add `src/adk_secure_sessions/rotation.py` with `rotate_encryption_keys()` and `RotationResult` dataclass; raw SQLAlchemy engine bypass of TypeDecorator with per-record `asyncio.to_thread()` crypto and `UPDATE WHERE update_time = :read_time` optimistic concurrency guard - Export `rotate_encryption_keys` and `RotationResult` from public API - Add ADR-009 documenting batch vs lazy rotation paths, no-coordinator decision, and `update_time` over `version` column rationale - Add `docs/how-to/key-rotation.md` operator guide covering both rotation paths with code examples and trade-offs - Add 10 unit tests (mocked engine) and 6 integration tests (real SQLite)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds a first-class, operator-facing key rotation path for encrypted session data, complementing the existing additional_backends lazy migration path and documenting the operational strategy in an ADR and How-To guide.
Changes:
- Introduce
rotate_encryption_keys(db_url, old_backend, new_backend) -> RotationResultto batch re-encrypt data acrosssessions,events,app_states, anduser_states. - Export the new rotation utility from the public package API and add unit + integration coverage.
- Document key rotation strategy and operations guidance; update MkDocs navigation to include the new pages/ADRs.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/adk_secure_sessions/rotation.py | Implements the async SQLAlchemy-based batch rotation utility and RotationResult. |
| src/adk_secure_sessions/init.py | Exports rotate_encryption_keys and RotationResult via imports + __all__. |
| tests/unit/test_rotation.py | Adds mocked-engine unit tests for rotation behaviors (filtering, concurrency, NULL handling, message safety). |
| tests/integration/test_key_rotation.py | Adds SQLite + real backend integration tests for end-to-end rotation lifecycle. |
| docs/how-to/key-rotation.md | Operator guide describing Path A (lazy) vs Path B (batch) rotation workflows. |
| docs/adr/ADR-009-key-rotation-strategy.md | ADR documenting the two-path strategy and design rationale (no coordinator, update_time guard). |
| mkdocs.yml | Wires the new How-To page and ADR entries into the docs nav. |
| .claude/rules/conventions.md | Adds explicit guidance about sync/async boundary review risk. |
| .claude/rules/dev-quality-checklist.md | Adds benchmark-claim hygiene items to the quality checklist. |
| _bmad/bmm/workflows/4-implementation/code-review/checklist.md | Adds a checklist item about numeric claims being measurement-derived. |
| _bmad-output/test-artifacts/test-review.md | Tracks story assignment note for an existing test-review recommendation. |
| _bmad-output/implementation-artifacts/sprint-status.yaml | Updates sprint generation metadata and story status for Epic 4 / Story 4.4. |
| _bmad-output/implementation-artifacts/4-4-zero-downtime-key-rotation.md | Story artifact capturing ACs, implementation notes, and mapping to tests/docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix idempotency overclaim: re-runs are only safe for cross-backend
rotation; for same-backend, a single pass is expected — document
clearly in docstring and how-to guide
- Change `raise DecryptionError(msg) from exc` to `from None` to
prevent key material leakage via exception chaining (NFR6); consistent
with existing codebase pattern in backends and type_decorator
- Add ciphertext guard to UPDATE WHERE clause: `AND {enc_col} = :old_val`
prevents silent overwrites when concurrent writes collide within the
same timestamp window (SQLite low-precision risk)
- Rename integration test to `test_can_rotate_back_between_fernet_backends`
with accurate docstring describing reverse-rotation scenario
- Add unit test verifying old_val is passed in UPDATE params (T011)
- Rename `## See Also` → `## Related` in how-to guide (doc convention)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d guarantee fixture teardown - Add UnicodeEncodeError to base64 decode except clause so corrupted non-ASCII TEXT rows raise DecryptionError instead of leaking the raw UnicodeEncodeError to callers - Wrap populated_old_key_db fixture setup in try/finally to guarantee svc.close() runs even if an append_event call raises - Add unit test (T012) for the non-ASCII → DecryptionError path
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Split Security Notes in how-to guide into Path A (zero-downtime) and Path B (requires service stop). Add pre-condition to rotation.py docstring. Rename test module docstring from "zero-downtime" to "batch".
Replace Dependabot with Renovate for dependency management, matching sister projects. Add uv-secure config to ignore GHSA-5239-wwwm-4pmq (Pygments ReDoS, low severity, no fix available). Selectively upgrade 7 transitive deps with known CVEs without bumping google-adk.
Three contracts: layered architecture (protocols/exceptions → serialization → backends/rotation → services), backend independence (fernet and aes_gcm don't cross-import), and rotation isolation (no backend or service imports). Runs in pre-commit and CI lint job.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before this PR, same-backend passphrase rotation (e.g., rotating a Fernet key) had no supported path —
additional_backendsraisesConfigurationErrorfor duplicatebackend_idvalues. This addsrotate_encryption_keys(), a standalone async utility that re-encrypts all session data directly via SQLAlchemy, bypassing the TypeDecorator to control which key decrypts.rotate_encryption_keys(db_url, old_backend, new_backend) -> RotationResultin newrotation.pymodule; reads raw base64 TEXT, parses envelope, decrypts withold_backend, re-encrypts withnew_backend, writes back withUPDATE WHERE update_time = :read_timeoptimistic concurrency guardRotationResult(rotated, skipped)dataclass; re-runs are idempotent (already-rotated records carry newbackend_idand are skipped silently)__init__.py__all__)update_timevsversioncolumn rationaledocs/how-to/key-rotation.mdoperator guide with Path A (lazy cross-backend viaadditional_backends) and Path B (batch viarotate_encryption_keys()) examplesTest:
uv run pytest tests/unit/test_rotation.py tests/integration/test_key_rotation.py -vPR Review
Checklist
uv run pytest)uv run ruff check .)!in title andBREAKING CHANGE:in bodyReview Focus
rotation.py:_rotate_table— raw SQL SELECT/UPDATE loop; per-recordasyncio.to_thread()crypto; optimistic concurrency viaupdate_time; events tablerowcount=0treated as cascade-deleted (not skipped)rotation.py:rotate_encryption_keys— four separate transactions (one per table); re-run idempotency contract in docstringevent_datatest, docstring idempotency/events semantics, and scalability note all addressed before this PRRelated
additional_backends)update_timeis available as concurrency guard