feat: batch #68, #72 — ENCRYPTION_KEY rotation + settings backup/recovery#85
Merged
Conversation
…very Closes #68 — POST /api/settings/rotate-key endpoint + UI flow for re-encrypting all stored API keys under a new ENCRYPTION_KEY. Proof-of- possession via old_key fingerprint match; transactional rollback on partial failure; .env update + restart instructed manually in response. Closes #72 — settings export/import + first-run backup warning. Export returns a schema_version=1 JSON envelope of all non-_meta.* Setting rows with the source ENCRYPTION_KEY fingerprint. Import is a 2-stage flow (/preview then /apply) gated on fingerprint match. Setup page shows an amber backup warning until the first save_key stamps a fingerprint. Service layer (src/services/settings_io.py): - export_settings_envelope, preview_import_envelope, apply_import_envelope - Caller owns the transaction; encrypted flag derived from key prefix (tamper-resistant — envelope flag is informational only) New helpers (src/services/crypto.py): - compute_fingerprint(key) — public proof-of-possession helper - META_KEY_PREFIX — canonical _meta.* declaration Frontend (src/templates/settings.html): - Shared Alpine modal reused by rotate-key and import-confirm - 2-stage import state machine with file size guard (1 MB) - Double-click guard on destructive actions Docs: docs/backup-and-recovery.md covers export/import/rotate procedure and error-code reference. README links to it. Tests: - tests/test_settings_io.py — 18 service-layer unit tests - tests/test_settings_api.py — 8 endpoint integration tests - Total: 26 new tests, 323 in suite all pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Implements ENCRYPTION_KEY rotation and a settings backup/recovery flow. Adds a new /api/settings/rotate-key endpoint (proof-of-possession via stored fingerprint + Fernet re-encryption of all api_key.% rows), a two-stage settings export/import flow (/api/settings/export, /api/settings/import/preview, /api/settings/import/apply) backed by a new settings_io service, supporting i18n strings, Alpine UI in settings.html, a first-run backup warning on the setup page, and operator documentation.
Changes:
- New
src/services/settings_io.pywith envelope export/preview/apply (schema_version=1,_meta.*filtering, fingerprint-gated import, encrypted-flag derived from key prefix). - New rotation + export/import endpoints in
src/api/settings.py, new error helpers insrc/errors.py, and a new publiccompute_fingerprinthelper plusMETA_KEY_PREFIXconstant insrc/services/crypto.py. - Settings UI section (rotate / export / import + confirmation modal), setup-page backup warning, en/ja i18n strings, README + new
docs/backup-and-recovery.md, and matching service/HTTP-layer tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/settings_io.py | New service implementing export envelope, preview, and apply with _meta.* filtering and prefix-derived encrypted flag. |
| src/services/crypto.py | Adds META_KEY_PREFIX and public compute_fingerprint; refactors get_fingerprint to use it. |
| src/api/settings.py | New /rotate-key, /export, /import/preview, /import/apply endpoints. |
| src/api/pages.py | Computes has_stamped_fingerprint to drive the setup-page backup warning. |
| src/errors.py | New AppError helpers for rotation, export, and import failure modes. |
| src/templates/settings.html | UI for Backup & Recovery section + confirmation modal + Alpine handlers. |
| src/templates/setup.html | First-run amber warning to back up ENCRYPTION_KEY. |
| src/i18n/{en,ja}.json | Adds strings for backup/recovery + setup warning; ja.json also converts many escape sequences to literal kana. |
| tests/test_settings_io.py | New service-layer tests for envelope shape, preview, apply, roundtrip. |
| tests/test_settings_api.py | New HTTP-layer tests for rotation and import/export endpoints. |
| README.md | Adds Backup & Recovery section linking to the new doc. |
| docs/backup-and-recovery.md | New operator documentation covering backup, restore, rotation, and error codes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+535
to
+581
| @router.post("/rotate-key") | ||
| async def rotate_encryption_key( | ||
| body: RotateKeyRequest, | ||
| session: AsyncSession = Depends(get_session), | ||
| ): | ||
| """Re-encrypt every api_key.% row from `old_key` to `new_key`. | ||
|
|
||
| Proof-of-possession: `old_key` must hash to the stored | ||
| `_meta.encryption_key_fingerprint`. If no fingerprint is stamped yet | ||
| (legacy/first-run), the check is skipped so the user can bootstrap. | ||
|
|
||
| Does NOT rewrite `.env`. The response carries an instruction telling the | ||
| operator to update `ENCRYPTION_KEY` in `.env` and restart the app | ||
| (Docker bind mounts and file-permission edge cases make in-process | ||
| `.env` rewrites brittle — see Q1 in the design discussion). | ||
|
|
||
| On partial failure, the session is rolled back and a 500 with | ||
| `ROTATION_PARTIAL_FAILURE` is returned; the stored fingerprint stays | ||
| stale so the existing rotation banner stays visible for residual rows. | ||
| """ | ||
| # Validate new_key format up front so we fail fast with a clean 400 | ||
| # instead of letting `reencrypt_all`'s Fernet constructor raise mid-loop. | ||
| try: | ||
| Fernet(body.new_key.encode()) | ||
| except Exception as e: | ||
| raise _rotation_invalid_new_key(str(e)) from e | ||
|
|
||
| service = EncryptionService(session) | ||
| stored_fp = await service.get_stored_fingerprint() | ||
| if stored_fp is not None and compute_fingerprint(body.old_key) != stored_fp: | ||
| raise rotation_wrong_key() | ||
|
|
||
| report = await service.reencrypt_all(body.old_key, body.new_key) | ||
|
|
||
| if report["failed"] > 0: | ||
| await session.rollback() | ||
| raise rotation_partial_failure(report["errors"]) | ||
|
|
||
| await session.commit() | ||
| return { | ||
| "success": report["success"], | ||
| "failed": 0, | ||
| "errors": [], | ||
| "instruction": ( | ||
| "Re-encryption complete. Update ENCRYPTION_KEY in your .env file to the new value and restart the app." | ||
| ), | ||
| } |
Comment on lines
+82
to
+84
| source_fp = envelope.get("source_fingerprint") | ||
| if not isinstance(source_fp, str) or not source_fp: | ||
| raise import_schema_unsupported(version) |
Comment on lines
+437
to
+438
| row = (await s.execute(select(Setting).where(Setting.key == "api_key.openai"))).scalar_one() | ||
| assert row.value == encrypt("sk-x") or row.value.startswith("gAAAAAB") |
Owner
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
Comment on lines
+76
to
+84
| def _validate_envelope(envelope: dict[str, Any]) -> None: | ||
| """Raise AppError on schema/fingerprint mismatch; no DB access.""" | ||
| version = envelope.get("schema_version") | ||
| if version not in SUPPORTED_SCHEMA_VERSIONS: | ||
| raise import_schema_unsupported(version) | ||
|
|
||
| source_fp = envelope.get("source_fingerprint") | ||
| if not isinstance(source_fp, str) or not source_fp: | ||
| raise import_schema_unsupported(version) |
| <p x-show="rotateForm.error" x-cloak class="mt-2 text-sm text-red-600 dark:text-red-400" x-text="rotateForm.error"></p> | ||
| <div x-show="rotateForm.result" x-cloak class="mt-3 p-3 rounded-md bg-green-50 dark:bg-green-900/30 border border-green-300 dark:border-green-700 text-green-800 dark:text-green-200 text-sm"> | ||
| <p class="font-medium mb-1" x-text="'{{ t('settings.rotate_success') }}'.replace('{n}', rotateForm.result?.success)"></p> | ||
| <p class="text-xs" x-text="rotateForm.result?.instruction"></p> |
| "export_button": "Download backup", | ||
| "import_title": "Import settings", | ||
| "import_desc": "Restore from a previously exported settings.json. Requires the same ENCRYPTION_KEY as the source instance.", | ||
| "import_select_file": "Select settings.json", |
Comment on lines
+437
to
+438
| row = (await s.execute(select(Setting).where(Setting.key == "api_key.openai"))).scalar_one() | ||
| assert row.value == encrypt("sk-x") or row.value.startswith("gAAAAAB") |
`_validate_envelope` previously let `EncryptionService.get_fingerprint()` raise a bare `RuntimeError` when `ENCRYPTION_KEY` was unset, surfacing as an unhandled 500 from `/import/preview` and `/import/apply`. `export_settings_envelope` already handled this with `export_fingerprint_missing()`; the import path now mirrors that. Adds `import_fingerprint_missing()` error factory (500 `IMPORT_FINGERPRINT_MISSING`) and a test (`test_raises_when_encryption_key_unset`) that toggles the env value manually around `isolated_settings()`'s teardown (which calls `encrypt()` and needs the key). Surfaced by /code-review's 75/100 borderline finding, addressed before merge so the asymmetry doesn't ship. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Closes #68
Closes #72
Summary
POST /api/settings/rotate-encryption-keyto re-encrypt every stored API key under a newENCRYPTION_KEY. Proof-of-possession via fingerprint match; partial-failure rollback leaves the rotation banner stale on purpose..envupdate + restart is operator-driven (response carries the instruction).GET /api/settings/export+POST /api/settings/import/{preview,apply}for disaster-recovery backups. Export is aschema_version=1JSON envelope of all non-_meta.*Setting rows; import is gated onsource_fingerprint == current_fingerprint. 2-stage flow so the operator confirms the row-count diff before any DB write.ENCRYPTION_KEYbefore they need to recover it.docs/backup-and-recovery.mdcovers the full procedure plus an error-code reference. README links to it.Implementation notes
src/services/settings_io.py(export_settings_envelope, preview_import_envelope, apply_import_envelope). HTTP-agnostic — caller owns the transaction. The_upsert_settingbypass is intentional and documented (mixing withreencrypt_allconfuses SQLAlchemy's identity map percrypto.py's transaction contract).encryptedflag on import is derived from the key prefix via_should_be_encrypted, not trusted from the envelope. Tamper-resistance: a forged envelope withencrypted=Falseon anapi_key.*row still lands as encrypted=True. Tested on both INSERT and UPDATE branches.src/services/crypto.py:compute_fingerprint(key)(public POP helper, dedupes inlinehashlib.sha256(...)) andMETA_KEY_PREFIX(canonical declaration, replaces stringly-typed"_meta."literals).AppErrorfactories insrc/errors.py: ROTATION_WRONG_KEY, ROTATION_INVALID_NEW_KEY, ROTATION_PARTIAL_FAILURE, EXPORT_FINGERPRINT_MISSING, IMPORT_FINGERPRINT_MISMATCH, IMPORT_SCHEMA_UNSUPPORTED.settings.*+ 2 insetup.*across bothen.jsonandja.json. Placeholder names align with field names ({rows_overwrite},{imported}etc.) to prevent silent translator drift.tests/test_settings_io.py+ 8 endpoint integration tests intests/test_settings_api.py. Full suite: 323 passed.Pre-PR review summary
Full reviews are saved in the plugin cache:
~/.claude/cache/gh-issue-driven/68-batch_68-72.gate1.md~/.claude/cache/gh-issue-driven/68-batch_68-72.gate2.md🤖 Generated via
/gh-issue-driven:ship