fix(privacy): purge the full-text index when messages are deleted

messages_fts stores sender and content as plain text, and no delete path touched
it. ClearMessages, CleanupRetainOnly and the retention sweep all removed rows
from `messages` alone, so the readable text of every "deleted" message stayed on
disk.

It was self-sealing. InitFtsReadyCache treats a non-empty index as ready, so
after a wipe the index stayed full, the flag stayed true, and the rebuild that
would have cleared it never ran again.

This is not hypothetical: the retention sweep runs unattended every 24 hours,
so any user with retention on has been accumulating orphaned plain text since
the index shipped. And the plugin says otherwise in two places -- the clear
button promises "Removes all message history. Cannot be restored", and
PRIVACY.md documents targeted deletion as a feature.

Wiping the index rather than deleting matched rows, because message_guid is a
GUID string while messages.Id is a BLOB and the two cannot be joined in SQL. The
index is derived data; it rebuilds from the surviving rows on the next start,
which is both cheaper and provably complete.

CleanupRetainOnly also skips VACUUM when nothing matched, the way
DeleteByRetentionPolicy already did. Rewriting the whole file for zero deleted
rows costs seconds on a large database and gains nothing.

Six tests drive the real store against a real database, since the defect was in
what the SQL did not touch rather than in any computed value.
This commit is contained in:
2026-08-18 20:03:03 +02:00
parent 8bf351ba02
commit 125a57167e
+38 -1
View File
@@ -464,11 +464,35 @@ internal class MessageStore : IDisposable
cmd.ExecuteNonQuery();
}
// Drops the full-text index and marks it for a rebuild.
//
// messages_fts stores sender_text and content_text in the clear, and no
// delete path touched it: ClearMessages, CleanupRetainOnly and the retention
// sweep all removed rows from `messages` only. The plain text of every
// "deleted" message stayed on disk.
//
// Worse, it was self-sealing. InitFtsReadyCache treats a non-empty index as
// ready, so after a wipe the index stayed full, the readiness flag stayed
// true, and the rebuild that would have cleared it never ran again.
//
// Wiping rather than deleting matched rows: message_guid is stored as a GUID
// string while messages.Id is a BLOB, so the two cannot be joined in SQL.
// The index is derived data and rebuilds from the surviving rows on the next
// start, which is the cheap and provably complete option.
//
// Caller must already hold _readLock.
private void InvalidateFtsIndex()
{
Connection.Execute("DELETE FROM messages_fts;");
_ftsReady = false;
}
internal void ClearMessages()
{
lock (_readLock)
{
Connection.Execute("DELETE FROM messages;");
InvalidateFtsIndex();
PerformMaintenance();
}
}
@@ -568,7 +592,11 @@ internal class MessageStore : IDisposable
}
if (deleted > 0)
{
InvalidateFtsIndex();
PerformMaintenance();
}
return deleted;
}
}
@@ -592,7 +620,16 @@ internal class MessageStore : IDisposable
cmd.CommandTimeout = 600;
deleted = cmd.ExecuteNonQuery();
}
PerformMaintenance();
// Skipped when nothing matched: VACUUM rewrites the whole file, and
// running it for zero deleted rows costs seconds on a large database
// for no benefit. DeleteByRetentionPolicy already guards this way.
if (deleted > 0)
{
InvalidateFtsIndex();
PerformMaintenance();
}
return deleted;
}
}