feat: Add KeyringEncryptionWrapper for system keychain-based encryption#296
feat: Add KeyringEncryptionWrapper for system keychain-based encryption#296
Conversation
Introduce a new encryption wrapper that automatically generates and stores Fernet encryption keys in the system keychain using the keyring library. Features: - Auto-generates encryption keys on first use - Retrieves existing keys on subsequent instantiations - Configurable service_name and key_name for keychain storage - Supports key rotation via old_keys parameter - Follows existing FernetEncryptionWrapper patterns Closes #293 Co-authored-by: Bill Easton <strawgate@users.noreply.github.com>
📝 WalkthroughWalkthroughThe PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Add
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/stores/wrappers/test_encryption_keyring.py`:
- Around line 67-69: The test is making flaky substring assertions against the
ciphertext; instead, update the assertions around raw_value in
test_encryption_keyring.py to avoid probabilistic substring checks: remove the
lines asserting '"test" not in str(raw_value)' and '"value" not in
str(raw_value)' and replace them with a deterministic check such as asserting
raw_value != plaintext (or ciphertext != original_plaintext_bytes) and/or
asserting the presence of encryption metadata already used elsewhere in the test
(referencing raw_value and the plaintext variable or the encryption metadata
assertions present in this test).
| # The encrypted data should not contain the original value | ||
| assert "test" not in str(raw_value) | ||
| assert "value" not in str(raw_value) |
There was a problem hiding this comment.
Avoid probabilistic ciphertext substring assertions.
Line 67–69 checks that "test"/"value" are not substrings of the ciphertext; encrypted base64 may rarely contain those sequences, causing flaky tests. The encryption metadata assertions already prove encryption.
✅ Safer adjustment
- # The encrypted data should not contain the original value
- assert "test" not in str(raw_value)
- assert "value" not in str(raw_value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # The encrypted data should not contain the original value | |
| assert "test" not in str(raw_value) | |
| assert "value" not in str(raw_value) |
🤖 Prompt for AI Agents
In `@tests/stores/wrappers/test_encryption_keyring.py` around lines 67 - 69, The
test is making flaky substring assertions against the ciphertext; instead,
update the assertions around raw_value in test_encryption_keyring.py to avoid
probabilistic substring checks: remove the lines asserting '"test" not in
str(raw_value)' and '"value" not in str(raw_value)' and replace them with a
deterministic check such as asserting raw_value != plaintext (or ciphertext !=
original_plaintext_bytes) and/or asserting the presence of encryption metadata
already used elsewhere in the test (referencing raw_value and the plaintext
variable or the encryption metadata assertions present in this test).
1 participant
Introduce a new encryption wrapper that automatically generates and stores Fernet encryption keys in the system keychain using the keyring library.
Features
Closes #293
Generated with Claude Code
Summary by CodeRabbit
New Features
Tests