fix(security): bookmarks/reminders stop storing decrypted text server-side
For encrypted rooms, bookmarks persist only {roomId, eventId, savedAt}
and reminders only their non-text fields; the preview, room name and
sender resolve locally at render/fire time from the timeline (with a
"Message unavailable" fallback). A one-time, loop-guarded cleanup strips
text from existing entries in currently-encrypted rooms. Unencrypted
rooms are unchanged. Unit-tested.
Fixes #10
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -103,7 +103,8 @@ function BookmarkItem({ bookmark, onJump, onRemove, preview, senderName }: Bookm
|
||||
const mx = useMatrixClient();
|
||||
const useAuthentication = useMediaAuthentication();
|
||||
const room = mx.getRoom(bookmark.roomId) ?? undefined;
|
||||
const displayRoomName = room?.name ?? bookmark.roomName;
|
||||
// E2EE-room bookmarks store no roomName; fall back past the '' placeholder.
|
||||
const displayRoomName = room?.name || bookmark.roomName || 'Unknown room';
|
||||
const avatarUrl = room
|
||||
? (getRoomAvatarUrl(mx, room, 96, useAuthentication) ?? undefined)
|
||||
: undefined;
|
||||
@@ -162,7 +163,7 @@ function BookmarkItem({ bookmark, onJump, onRemove, preview, senderName }: Bookm
|
||||
style={{ justifyContent: 'flex-start', height: 'unset', padding: config.space.S200 }}
|
||||
>
|
||||
<Text className={css.BookmarkPreview} size="T200" priority="400">
|
||||
{preview ?? (bookmark.previewText || '(no preview)')}
|
||||
{preview ?? (bookmark.previewText || 'Message unavailable')}
|
||||
</Text>
|
||||
</Button>
|
||||
</Box>
|
||||
@@ -173,13 +174,16 @@ type LiveBookmarkItemProps = BookmarkItemProps & { room: Room };
|
||||
|
||||
// Renders the same layout as BookmarkItem, but resolves the message body live so
|
||||
// edits (m.replace, applied by useRoomEvent) and redactions are reflected. The
|
||||
// stored snapshot (previewText) remains the fallback for loading/failed/empty states.
|
||||
// stored snapshot (previewText) remains the fallback for loading/failed/empty
|
||||
// states; bookmarks from E2EE rooms have no snapshot at all (account data is
|
||||
// server-readable), so the live event is their only source of text.
|
||||
function LiveBookmarkItem({ room, bookmark, onJump, onRemove }: LiveBookmarkItemProps) {
|
||||
const liveEvent = useRoomEvent(room, bookmark.eventId, () =>
|
||||
room.findEventById(bookmark.eventId),
|
||||
);
|
||||
|
||||
const snapshot = bookmark.previewText || '(no preview)';
|
||||
const snapshot =
|
||||
bookmark.previewText || (liveEvent === undefined ? 'Loading…' : 'Message unavailable');
|
||||
let preview: ReactNode = snapshot;
|
||||
|
||||
// undefined (loading) and null (fetch failed / not found) both keep the snapshot.
|
||||
@@ -234,7 +238,7 @@ function RoomGroupHeader({
|
||||
const mx = useMatrixClient();
|
||||
const useAuthentication = useMediaAuthentication();
|
||||
const room = mx.getRoom(roomId) ?? undefined;
|
||||
const displayRoomName = room?.name ?? roomName;
|
||||
const displayRoomName = room?.name || roomName || 'Unknown room';
|
||||
const avatarUrl = room
|
||||
? (getRoomAvatarUrl(mx, room, 96, useAuthentication) ?? undefined)
|
||||
: undefined;
|
||||
@@ -327,13 +331,20 @@ export function BookmarksPanel({ onClose }: BookmarksPanelProps) {
|
||||
() =>
|
||||
query.length === 0
|
||||
? bookmarks
|
||||
: bookmarks.filter(
|
||||
(bk) =>
|
||||
bk.previewText.toLowerCase().includes(query) ||
|
||||
: bookmarks.filter((bk) => {
|
||||
// E2EE-room bookmarks have no stored text: match against the locally
|
||||
// cached event body / live room name instead (nothing is fetched here).
|
||||
const room = mx.getRoom(bk.roomId);
|
||||
const localBody = room?.findEventById(bk.eventId)?.getContent()?.body;
|
||||
return (
|
||||
(bk.previewText?.toLowerCase().includes(query) ?? false) ||
|
||||
(typeof localBody === 'string' && localBody.toLowerCase().includes(query)) ||
|
||||
bk.roomName.toLowerCase().includes(query) ||
|
||||
(bk.senderName?.toLowerCase().includes(query) ?? false),
|
||||
),
|
||||
[bookmarks, query],
|
||||
(room?.name.toLowerCase().includes(query) ?? false) ||
|
||||
(bk.senderName?.toLowerCase().includes(query) ?? false)
|
||||
);
|
||||
}),
|
||||
[mx, bookmarks, query],
|
||||
);
|
||||
|
||||
// Prune collapsed roomIds that no longer have any bookmark, so a room re-saved
|
||||
|
||||
@@ -1273,6 +1273,9 @@ export const Message = React.memo(
|
||||
const content = mEvent.getContent();
|
||||
const body: string =
|
||||
(content?.body as string | undefined) ?? '';
|
||||
// For E2EE rooms useBookmarks strips the text
|
||||
// fields before persisting (account data is
|
||||
// server-readable); the panel resolves them live.
|
||||
addBookmark({
|
||||
roomId: room.roomId,
|
||||
eventId,
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {
|
||||
Bookmark,
|
||||
cleanupEncryptedBookmarks,
|
||||
hasBookmarkText,
|
||||
stripBookmarkText,
|
||||
toStorableBookmark,
|
||||
} from './useBookmarks';
|
||||
import { cleanupEncryptedReminders, Reminder, toStorableReminder } from './useReminders';
|
||||
|
||||
// E2EE policy for bookmarks/reminders (Gitea #10): account data is stored
|
||||
// unencrypted on the homeserver, so entries for encrypted rooms must carry no
|
||||
// message text or names. `isEncryptedRoom` is injected to keep these pure.
|
||||
const isEncrypted = (roomId: string) => roomId === '!enc';
|
||||
|
||||
const full = (roomId: string): Bookmark => ({
|
||||
roomId,
|
||||
eventId: `$ev-${roomId}`,
|
||||
savedAt: 100,
|
||||
previewText: 'secret text',
|
||||
roomName: 'Room',
|
||||
senderName: 'Alice',
|
||||
});
|
||||
|
||||
test('stripBookmarkText keeps only roomId/eventId/savedAt', () => {
|
||||
const stripped = stripBookmarkText(full('!enc'));
|
||||
assert.deepEqual(stripped, { roomId: '!enc', eventId: '$ev-!enc', savedAt: 100, roomName: '' });
|
||||
assert.equal(hasBookmarkText(stripped), false);
|
||||
assert.equal(hasBookmarkText(full('!enc')), true);
|
||||
});
|
||||
|
||||
test('toStorableBookmark strips text for encrypted rooms only', () => {
|
||||
const plain = full('!plain');
|
||||
assert.equal(toStorableBookmark(plain, isEncrypted), plain);
|
||||
const enc = toStorableBookmark(full('!enc'), isEncrypted);
|
||||
assert.equal(enc.previewText, undefined);
|
||||
assert.equal(enc.senderName, undefined);
|
||||
assert.equal(enc.roomName, '');
|
||||
});
|
||||
|
||||
test('cleanupEncryptedBookmarks returns undefined when nothing to strip', () => {
|
||||
const already = stripBookmarkText(full('!enc'));
|
||||
assert.equal(cleanupEncryptedBookmarks([full('!plain'), already], isEncrypted), undefined);
|
||||
assert.equal(cleanupEncryptedBookmarks([], isEncrypted), undefined);
|
||||
});
|
||||
|
||||
test('cleanupEncryptedBookmarks strips legacy encrypted entries and keeps the rest', () => {
|
||||
const plain = full('!plain');
|
||||
const out = cleanupEncryptedBookmarks([plain, full('!enc')], isEncrypted);
|
||||
assert.ok(out);
|
||||
assert.equal(out[0], plain);
|
||||
assert.deepEqual(out[1], stripBookmarkText(full('!enc')));
|
||||
// Running it again on the result is a no-op (no write loop).
|
||||
assert.equal(cleanupEncryptedBookmarks(out, isEncrypted), undefined);
|
||||
});
|
||||
|
||||
const reminder = (roomId: string): Reminder => ({
|
||||
roomId,
|
||||
eventId: `$ev-${roomId}`,
|
||||
timestamp: 200,
|
||||
message: 'secret text',
|
||||
});
|
||||
|
||||
test('toStorableReminder drops message for encrypted rooms only', () => {
|
||||
const plain = reminder('!plain');
|
||||
assert.equal(toStorableReminder(plain, isEncrypted), plain);
|
||||
assert.deepEqual(toStorableReminder(reminder('!enc'), isEncrypted), {
|
||||
roomId: '!enc',
|
||||
eventId: '$ev-!enc',
|
||||
timestamp: 200,
|
||||
});
|
||||
});
|
||||
|
||||
test('cleanupEncryptedReminders strips legacy entries once', () => {
|
||||
const out = cleanupEncryptedReminders([reminder('!plain'), reminder('!enc')], isEncrypted);
|
||||
assert.ok(out);
|
||||
assert.equal(out[0].message, 'secret text');
|
||||
assert.equal(out[1].message, undefined);
|
||||
assert.equal(cleanupEncryptedReminders(out, isEncrypted), undefined);
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import { useCallback } from 'react';
|
||||
import { useCallback, useEffect } from 'react';
|
||||
import { MatrixClient } from 'matrix-js-sdk';
|
||||
import { useMatrixClient } from './useMatrixClient';
|
||||
import { createAccountDataListStore } from './createAccountDataListStore';
|
||||
|
||||
@@ -6,7 +7,14 @@ export type Bookmark = {
|
||||
roomId: string;
|
||||
eventId: string;
|
||||
savedAt: number;
|
||||
previewText: string;
|
||||
// Snapshot fields. Account data is stored UNENCRYPTED on the homeserver, so
|
||||
// for E2EE rooms none of these are persisted (see stripBookmarkText) — the
|
||||
// panel resolves the preview / room / author live from the local timeline
|
||||
// instead. They remain for unencrypted rooms (already server-visible) and for
|
||||
// entries saved before this policy, so older stored bookmarks still render.
|
||||
previewText?: string;
|
||||
// Kept as a (possibly empty) string in memory because the grouping helpers in
|
||||
// utils/bookmarks read it directly; the store omits it from the wire when empty.
|
||||
roomName: string;
|
||||
// Author display name, snapshotted at save time. Optional for backward
|
||||
// compatibility with bookmarks stored before attribution was added; the panel
|
||||
@@ -16,22 +24,83 @@ export type Bookmark = {
|
||||
senderName?: string;
|
||||
};
|
||||
|
||||
// Wire shape: identical to Bookmark except that `roomName` may be absent.
|
||||
type StoredBookmark = Omit<Bookmark, 'roomName'> & { roomName?: string };
|
||||
|
||||
const BOOKMARKS_KEY = 'io.lotus.bookmarks';
|
||||
const MAX_BOOKMARKS = 500;
|
||||
|
||||
type BookmarksContent = {
|
||||
bookmarks: Bookmark[];
|
||||
bookmarks: StoredBookmark[];
|
||||
};
|
||||
|
||||
/** True when the entry carries any of the text fields we refuse to upload for E2EE rooms. */
|
||||
export const hasBookmarkText = (b: Bookmark): boolean =>
|
||||
b.previewText !== undefined || b.roomName !== '' || b.senderName !== undefined;
|
||||
|
||||
/**
|
||||
* Reduce a bookmark to its non-text fields ({roomId, eventId, savedAt}). Used for
|
||||
* messages in encrypted rooms so no decrypted plaintext (or who said it) ever
|
||||
* reaches the server's account-data store.
|
||||
*/
|
||||
export const stripBookmarkText = (b: Bookmark): Bookmark => ({
|
||||
roomId: b.roomId,
|
||||
eventId: b.eventId,
|
||||
savedAt: b.savedAt,
|
||||
roomName: '',
|
||||
});
|
||||
|
||||
/**
|
||||
* Apply the E2EE storage policy to a bookmark about to be persisted.
|
||||
* `isEncryptedRoom` is injected so the policy is testable without a client.
|
||||
*/
|
||||
export const toStorableBookmark = (
|
||||
b: Bookmark,
|
||||
isEncryptedRoom: (roomId: string) => boolean,
|
||||
): Bookmark => (isEncryptedRoom(b.roomId) ? stripBookmarkText(b) : b);
|
||||
|
||||
/**
|
||||
* One-time cleanup for entries persisted before the E2EE policy: returns the list
|
||||
* with text fields stripped for every bookmark whose room is currently encrypted,
|
||||
* or `undefined` when nothing needs to change (so callers can skip the write).
|
||||
*/
|
||||
export const cleanupEncryptedBookmarks = (
|
||||
bookmarks: Bookmark[],
|
||||
isEncryptedRoom: (roomId: string) => boolean,
|
||||
): Bookmark[] | undefined => {
|
||||
let changed = false;
|
||||
const next = bookmarks.map((b) => {
|
||||
if (hasBookmarkText(b) && isEncryptedRoom(b.roomId)) {
|
||||
changed = true;
|
||||
return stripBookmarkText(b);
|
||||
}
|
||||
return b;
|
||||
});
|
||||
return changed ? next : undefined;
|
||||
};
|
||||
|
||||
const isEncryptedRoomFor =
|
||||
(mx: MatrixClient) =>
|
||||
(roomId: string): boolean =>
|
||||
mx.getRoom(roomId)?.hasEncryptionStateEvent() ?? false;
|
||||
|
||||
// Shared, concurrency-safe store. See createAccountDataListStore for why the
|
||||
// snapshot + write queue must be module-scoped (writes are serialized to avoid
|
||||
// lost updates, since setAccountData replaces the whole content with no merge).
|
||||
const bookmarksStore = createAccountDataListStore<Bookmark[], BookmarksContent>({
|
||||
eventType: BOOKMARKS_KEY,
|
||||
read: (content) => content?.bookmarks ?? [],
|
||||
write: (bookmarks) => ({ bookmarks }),
|
||||
read: (content) => (content?.bookmarks ?? []).map((b) => ({ ...b, roomName: b.roomName ?? '' })),
|
||||
write: (bookmarks) => ({
|
||||
// Drop the in-memory '' placeholder so stripped entries stay {roomId, eventId, savedAt}.
|
||||
bookmarks: bookmarks.map(({ roomName, ...rest }) => (roomName ? { ...rest, roomName } : rest)),
|
||||
}),
|
||||
});
|
||||
|
||||
// Guard so the load-time cleanup below runs once per client, not on every mount
|
||||
// (useBookmarks is mounted by every message row) and not again after its own
|
||||
// write echoes back.
|
||||
let cleanedUpFor: MatrixClient | null = null;
|
||||
|
||||
export function useBookmarks(): {
|
||||
bookmarks: Bookmark[];
|
||||
addBookmark: (b: Bookmark) => Promise<void>;
|
||||
@@ -41,12 +110,33 @@ export function useBookmarks(): {
|
||||
const mx = useMatrixClient();
|
||||
const bookmarks = bookmarksStore.useValue(mx);
|
||||
|
||||
// Strip text from entries saved (by older builds) for rooms that are encrypted,
|
||||
// writing the store back at most once per client.
|
||||
useEffect(() => {
|
||||
if (cleanedUpFor === mx) return;
|
||||
cleanedUpFor = mx;
|
||||
const cleaned = cleanupEncryptedBookmarks(bookmarksStore.getLatest(mx), isEncryptedRoomFor(mx));
|
||||
if (cleaned) {
|
||||
bookmarksStore
|
||||
.enqueueWrite(
|
||||
mx,
|
||||
(current) => cleanupEncryptedBookmarks(current, isEncryptedRoomFor(mx)) ?? current,
|
||||
)
|
||||
.catch(() => {
|
||||
// Retry on the next client (re)mount.
|
||||
if (cleanedUpFor === mx) cleanedUpFor = null;
|
||||
});
|
||||
}
|
||||
}, [mx]);
|
||||
|
||||
const addBookmark = useCallback(
|
||||
(b: Bookmark) =>
|
||||
bookmarksStore.enqueueWrite(mx, (current) => {
|
||||
// Never upload plaintext for E2EE rooms — account data is server-readable.
|
||||
const stored = toStorableBookmark(b, isEncryptedRoomFor(mx));
|
||||
// Avoid duplicates
|
||||
const filtered = current.filter((bk) => bk.eventId !== b.eventId);
|
||||
let next = [b, ...filtered];
|
||||
const filtered = current.filter((bk) => bk.eventId !== stored.eventId);
|
||||
let next = [stored, ...filtered];
|
||||
if (next.length > MAX_BOOKMARKS) {
|
||||
next = next.slice(0, MAX_BOOKMARKS);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { useCallback } from 'react';
|
||||
import { useCallback, useEffect } from 'react';
|
||||
import { MatrixClient } from 'matrix-js-sdk';
|
||||
import { useMatrixClient } from './useMatrixClient';
|
||||
import { createAccountDataListStore } from './createAccountDataListStore';
|
||||
|
||||
@@ -6,7 +7,10 @@ export type Reminder = {
|
||||
roomId: string;
|
||||
eventId: string;
|
||||
timestamp: number;
|
||||
message: string;
|
||||
// Message preview. Account data is stored UNENCRYPTED on the homeserver, so
|
||||
// this is omitted for E2EE rooms (see stripReminderText) and the toast resolves
|
||||
// the text locally at fire time. Optional for entries stored before that policy.
|
||||
message?: string;
|
||||
};
|
||||
|
||||
const REMINDERS_KEY = 'io.lotus.reminders';
|
||||
@@ -15,6 +19,43 @@ type RemindersContent = {
|
||||
reminders: Reminder[];
|
||||
};
|
||||
|
||||
/** Reduce a reminder to its non-text fields ({roomId, eventId, timestamp}). */
|
||||
export const stripReminderText = (r: Reminder): Reminder => ({
|
||||
roomId: r.roomId,
|
||||
eventId: r.eventId,
|
||||
timestamp: r.timestamp,
|
||||
});
|
||||
|
||||
/** Apply the E2EE storage policy to a reminder about to be persisted. */
|
||||
export const toStorableReminder = (
|
||||
r: Reminder,
|
||||
isEncryptedRoom: (roomId: string) => boolean,
|
||||
): Reminder => (isEncryptedRoom(r.roomId) ? stripReminderText(r) : r);
|
||||
|
||||
/**
|
||||
* One-time cleanup for entries persisted before the E2EE policy; `undefined`
|
||||
* when nothing needs to change (so callers can skip the write).
|
||||
*/
|
||||
export const cleanupEncryptedReminders = (
|
||||
reminders: Reminder[],
|
||||
isEncryptedRoom: (roomId: string) => boolean,
|
||||
): Reminder[] | undefined => {
|
||||
let changed = false;
|
||||
const next = reminders.map((r) => {
|
||||
if (r.message !== undefined && isEncryptedRoom(r.roomId)) {
|
||||
changed = true;
|
||||
return stripReminderText(r);
|
||||
}
|
||||
return r;
|
||||
});
|
||||
return changed ? next : undefined;
|
||||
};
|
||||
|
||||
const isEncryptedRoomFor =
|
||||
(mx: MatrixClient) =>
|
||||
(roomId: string): boolean =>
|
||||
mx.getRoom(roomId)?.hasEncryptionStateEvent() ?? false;
|
||||
|
||||
// Shared, concurrency-safe store. See createAccountDataListStore for why the
|
||||
// snapshot + write queue must be module-scoped (writes are serialized to avoid
|
||||
// lost updates, since setAccountData replaces the whole content with no merge).
|
||||
@@ -24,6 +65,9 @@ const remindersStore = createAccountDataListStore<Reminder[], RemindersContent>(
|
||||
write: (reminders) => ({ reminders }),
|
||||
});
|
||||
|
||||
// Once-per-client guard for the load-time cleanup (mirrors useBookmarks).
|
||||
let cleanedUpFor: MatrixClient | null = null;
|
||||
|
||||
export function useReminders(): {
|
||||
reminders: Reminder[];
|
||||
addReminder: (r: Reminder) => Promise<void>;
|
||||
@@ -33,8 +77,30 @@ export function useReminders(): {
|
||||
const mx = useMatrixClient();
|
||||
const reminders = remindersStore.useValue(mx);
|
||||
|
||||
// Strip text from entries saved (by older builds) for rooms that are encrypted,
|
||||
// writing the store back at most once per client.
|
||||
useEffect(() => {
|
||||
if (cleanedUpFor === mx) return;
|
||||
cleanedUpFor = mx;
|
||||
if (cleanupEncryptedReminders(remindersStore.getLatest(mx), isEncryptedRoomFor(mx))) {
|
||||
remindersStore
|
||||
.enqueueWrite(
|
||||
mx,
|
||||
(current) => cleanupEncryptedReminders(current, isEncryptedRoomFor(mx)) ?? current,
|
||||
)
|
||||
.catch(() => {
|
||||
if (cleanedUpFor === mx) cleanedUpFor = null;
|
||||
});
|
||||
}
|
||||
}, [mx]);
|
||||
|
||||
const addReminder = useCallback(
|
||||
(r: Reminder) => remindersStore.enqueueWrite(mx, (current) => [...current, r]),
|
||||
(r: Reminder) =>
|
||||
remindersStore.enqueueWrite(mx, (current) => [
|
||||
...current,
|
||||
// Never upload plaintext for E2EE rooms — account data is server-readable.
|
||||
toStorableReminder(r, isEncryptedRoomFor(mx)),
|
||||
]),
|
||||
[mx],
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user