fix(store): fail closed on decrypt failure in decrypt_text_at_rest #264

2026-07-31T16:04:45Z

KofTwentyTwo commented

2026-07-31 16:04:45 +00:00

(Migrated from github.com)

Summary

PayloadCipher::decrypt_text_at_rest previously returned "" on ANY failure (hex-decode / AEAD-decrypt / UTF-8), so a corrupted or tampered body_plain/body_html column read back as an empty body instead of surfacing an error -- defeating the AEAD's integrity guarantee exactly where it matters: detecting tampering on read. This is the read-path twin of #198/#236, which fixed the write path (encrypt_text_at_rest).
Changed decrypt_text_at_rest to return Result<String, CipherError>. An empty stored ciphertext still legitimately short-circuits to Ok(String::new()); hex-decode, AEAD-decrypt, and UTF-8 failures now propagate as Err.
Added DatabaseError::Decryption(String) (mirroring the existing Encryption variant) and propagated it through every read call site in db.rs: backfill_message_fts, list_messages, get_message, get_message_chunk, list_messages_for_export.

Test plan

cargo fmt --all -- --check
cargo check-all (clippy --all-targets -D warnings, 0 warnings)
cargo test-all (0 ignored) -- all green, including new tests:
- cipher::tests::text_at_rest_tampered_ciphertext_fails_closed
- cipher::tests::text_at_rest_invalid_hex_fails_closed
- cipher::tests::text_at_rest_wrong_key_fails_closed
- db::tests::corrupted_body_ciphertext_fails_closed_on_read -- corrupts a stored ciphertext column after a legitimate write and proves get_message/list_messages return Err(DatabaseError::Decryption), not an empty body
Existing encryption/decryption round-trip tests remain green, proving genuinely empty bodies still round-trip as Ok("") with no false positive

Refs #237 #162

## Summary - `PayloadCipher::decrypt_text_at_rest` previously returned `""` on ANY failure (hex-decode / AEAD-decrypt / UTF-8), so a corrupted or tampered `body_plain`/`body_html` column read back as an empty body instead of surfacing an error -- defeating the AEAD's integrity guarantee exactly where it matters: detecting tampering on read. This is the read-path twin of #198/#236, which fixed the write path (`encrypt_text_at_rest`). - Changed `decrypt_text_at_rest` to return `Result<String, CipherError>`. An empty stored ciphertext still legitimately short-circuits to `Ok(String::new())`; hex-decode, AEAD-decrypt, and UTF-8 failures now propagate as `Err`. - Added `DatabaseError::Decryption(String)` (mirroring the existing `Encryption` variant) and propagated it through every read call site in `db.rs`: `backfill_message_fts`, `list_messages`, `get_message`, `get_message_chunk`, `list_messages_for_export`. ## Test plan - [x] `cargo fmt --all -- --check` - [x] `cargo check-all` (clippy `--all-targets` `-D warnings`, 0 warnings) - [x] `cargo test-all` (0 ignored) -- all green, including new tests: - `cipher::tests::text_at_rest_tampered_ciphertext_fails_closed` - `cipher::tests::text_at_rest_invalid_hex_fails_closed` - `cipher::tests::text_at_rest_wrong_key_fails_closed` - `db::tests::corrupted_body_ciphertext_fails_closed_on_read` -- corrupts a stored ciphertext column after a legitimate write and proves `get_message`/`list_messages` return `Err(DatabaseError::Decryption)`, not an empty body - [x] Existing encryption/decryption round-trip tests remain green, proving genuinely empty bodies still round-trip as `Ok("")` with no false positive Refs #237 #162

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#264