Decide "is this encrypted" by decrypting, not by sniffing a byte

[Cipher208]

Follow-up to #89, which reported this and left it alone because it changes a public
helper's contract. You said fix it.

The bug

is_encrypted_blob asked a security question and answered it from one byte.

The envelope is nonce(24) || ciphertext, so its first byte is the first byte of a
random nonce — {, [, space or newline in 4 cases out of 256. The old test
was head not in (b"{", b"[", b" ", b"\n"), so real, readable ciphertext was called
"plain JSON". Measured over 4096 draws: 69 misclassifications, 1.68%.

That is the wrong direction for this predicate to fail — it turns a readable secret
into a missing one.

The fix

Detection attempts decryption. A blob is encrypted when it decrypts under the given
key, and nothing else counts. Truncated blobs, filler bytes, plain JSON, tampered
ciphertext and blobs encrypted under a foreign key all report False, each for the
right reason.

Breaking, and why it should be

shared/crypto.py: is_encrypted_blob(blob_head: bytes) → is_encrypted_blob(blob: bytes, master_key: bytes).

A one-byte prefix cannot be authenticated, so the old signature was incapable of a
correct answer. Keeping it would mean keeping the lie. The path-based helpers —
shared.master_key.is_encrypted_blob(path) and shared.saga.impl.crypto.is_encrypted_blob(path)
— keep their signatures and are decrypt-based now too.

Nothing inside the package used any of these to decide anything: the saga read path
already decrypted first. They are exported utilities, which is exactly why the wrong
answer mattered for callers rather than for us.

The workaround this bug had accumulated

Two tests had adapted to the bug instead of fixing it, and both are now honest:

• tests/test_shared/test_crypto_smoke.py redrew up to 32 blobs until one looked
  encrypted, because a single draw flaked about 1.6% of runs. It now asserts the
  opposite: it waits for the {-first nonce and requires detection to survive it.
• tests/test_secrets.py proved encryption with filler bytes (b"\xab\xcd" * 30),
  which only ever passed because anything not { counted as encrypted. It writes a real
  envelope now.

Verification

• Red/green on one live blob: with a {-first nonce the old logic returns False
  for a blob that decrypts; the new one returns True.
• Full suite 2089 passed (+2 new), run by the pre-commit gate.
• ruff check, ruff format --check, mypy — clean.

Not mine, reported instead

tests/test_hypothesis.py::{TestSagaCompensationLogic::test_compensation_reverts_core_memory,TestMemoryStateMachine::test_remember_forget_invariant}
fail when selected in isolation (sqlite3.OperationalError: no such table: core_memory)
but pass in the full run. Verified pre-existing: they fail identically on master with
these changes stashed. That is test-order dependence, unrelated to this work, and left
alone.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Repository: Cipher208/a-memory/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 392a7eea-d13b-4bb0-9247-bf91245264f9

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}