fix(security): persistent search index forgets redacted and left-room text
Nothing ever removed an indexed row: redacted messages stayed searchable with full plaintext and rendered as normal results. Now: a client-level RoomEvent.Redaction listener deletes the row, leave/ban clears the room (clearRoom finally has a caller), m.replace edits upsert the original row instead of indexing the "* fallback" separately, and cached rows whose local event is redacted render through the existing redacted_because placeholder. Unit-tested. Fixes #14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -0,0 +1,56 @@
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { SearchCacheRow } from '../../utils/searchCache';
|
||||
|
||||
// useLocalMessageSearch.ts imports searchCacheEnabledAtom, which touches
|
||||
// localStorage at module-load time (atomWithLocalStorage reads the initial
|
||||
// value eagerly). Stub it before a dynamic import — a static import would
|
||||
// hoist above the stub. Same pattern as state/plaintextCaches.test.ts.
|
||||
(globalThis as { localStorage?: unknown }).localStorage = {
|
||||
getItem: () => null,
|
||||
setItem: () => {},
|
||||
removeItem: () => {},
|
||||
};
|
||||
|
||||
const { rowToResultItem } = await import('./useLocalMessageSearch');
|
||||
|
||||
const row = (overrides: Partial<SearchCacheRow> = {}): SearchCacheRow => ({
|
||||
roomId: '!r1',
|
||||
eventId: '$1',
|
||||
ts: 100,
|
||||
sender: '@a',
|
||||
body: 'hello world',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
// Gitea #14 — cached rows for a locally-known-redacted event must carry a
|
||||
// `redacted_because` marker so SearchResultGroup's guard renders the
|
||||
// "message deleted" placeholder instead of the stale plaintext.
|
||||
|
||||
test('rowToResultItem: plain row has no redacted_because marker', () => {
|
||||
const item = rowToResultItem(row());
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
assert.equal((item.event as any).unsigned?.redacted_because, undefined);
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
assert.equal((item.event as any).content.body, 'hello world');
|
||||
});
|
||||
|
||||
test('rowToResultItem: redacted=true sets the redacted_because marker', () => {
|
||||
const item = rowToResultItem(row(), true);
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
assert.ok((item.event as any).unsigned?.redacted_because);
|
||||
});
|
||||
|
||||
test('rowToResultItem: falls back to pollText when body is empty', () => {
|
||||
const item = rowToResultItem(row({ body: '', pollText: 'question answer' }));
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
assert.equal((item.event as any).content.body, 'question answer');
|
||||
});
|
||||
|
||||
test('rowToResultItem: carries formattedBody as HTML when present', () => {
|
||||
const item = rowToResultItem(row({ formattedBody: '<b>hi</b>' }));
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const content = (item.event as any).content;
|
||||
assert.equal(content.format, 'org.matrix.custom.html');
|
||||
assert.equal(content.formatted_body, '<b>hi</b>');
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { EventType, MatrixEvent } from 'matrix-js-sdk';
|
||||
import { EventType, MatrixEvent, RelationType } from 'matrix-js-sdk';
|
||||
import { useCallback } from 'react';
|
||||
import { useAtomValue } from 'jotai';
|
||||
import { useMatrixClient } from '../../hooks/useMatrixClient';
|
||||
@@ -38,13 +38,15 @@ type ExtractedText = {
|
||||
const POLL_START_TYPES = ['m.poll.start', 'org.matrix.msc3381.poll.start'];
|
||||
|
||||
/**
|
||||
* Pull the text we index/search from a decrypted event's content. Returns
|
||||
* `null` for events that carry no searchable text (e.g. stickers).
|
||||
* Pull the text we index/search from an event type + content pair. Returns
|
||||
* `null` when there's no searchable text (e.g. stickers). Split out from
|
||||
* `extractText` so an edit's `m.new_content` can be run through the same
|
||||
* logic as a normal event's content.
|
||||
*/
|
||||
const extractText = (event: MatrixEvent): ExtractedText | null => {
|
||||
const evType = event.getType();
|
||||
const content = event.getContent();
|
||||
|
||||
const extractTextFromContent = (
|
||||
evType: string,
|
||||
content: Record<string, unknown>,
|
||||
): ExtractedText | null => {
|
||||
if (POLL_START_TYPES.includes(evType)) {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const poll = (content['m.poll'] ?? content['org.matrix.msc3381.poll.start']) as any;
|
||||
@@ -74,6 +76,13 @@ const extractText = (event: MatrixEvent): ExtractedText | null => {
|
||||
return { body, formattedBody, pollText: '' };
|
||||
};
|
||||
|
||||
/**
|
||||
* Pull the text we index/search from a decrypted event's content. Returns
|
||||
* `null` for events that carry no searchable text (e.g. stickers).
|
||||
*/
|
||||
const extractText = (event: MatrixEvent): ExtractedText | null =>
|
||||
extractTextFromContent(event.getType(), event.getContent());
|
||||
|
||||
/** Does the extracted text contain the (already-lowercased) term? */
|
||||
const matchesTerm = (text: ExtractedText, termLower: string): boolean =>
|
||||
text.body.toLowerCase().includes(termLower) ||
|
||||
@@ -85,8 +94,17 @@ const rowMatchesTerm = (row: SearchCacheRow, termLower: string): boolean =>
|
||||
(row.formattedBody ?? '').toLowerCase().includes(termLower) ||
|
||||
(row.pollText ?? '').toLowerCase().includes(termLower);
|
||||
|
||||
/** Build the synthetic result item a cached row renders as (text message). */
|
||||
const rowToResultItem = (row: SearchCacheRow): ResultItem => {
|
||||
/**
|
||||
* Build the synthetic result item a cached row renders as (text message).
|
||||
*
|
||||
* `redacted` marks a row whose event we can tell, from the local timeline,
|
||||
* has since been redacted (the async cache-delete listener in
|
||||
* `searchCacheInvalidation.ts` may not have caught up yet). It carries a
|
||||
* `redacted_because` marker on `unsigned` so `SearchResultGroup`'s existing
|
||||
* guard renders the "message deleted" placeholder instead of the stale
|
||||
* plaintext (Gitea #14).
|
||||
*/
|
||||
export const rowToResultItem = (row: SearchCacheRow, redacted = false): ResultItem => {
|
||||
const bodyText = row.body || row.pollText || '';
|
||||
const content: Record<string, unknown> = { msgtype: 'm.text', body: bodyText };
|
||||
if (row.formattedBody) {
|
||||
@@ -100,7 +118,7 @@ const rowToResultItem = (row: SearchCacheRow): ResultItem => {
|
||||
sender: row.sender,
|
||||
origin_server_ts: row.ts,
|
||||
content,
|
||||
unsigned: {},
|
||||
unsigned: redacted ? { redacted_because: { content: {} } } : {},
|
||||
};
|
||||
return {
|
||||
rank: 0,
|
||||
@@ -196,16 +214,43 @@ export const useLocalMessageSearch = () => {
|
||||
|
||||
// Persist every indexable (text-bearing) event we scanned, regardless
|
||||
// of whether it matches the current term — future searches benefit.
|
||||
if (cacheEnabled && text && event.getId()) {
|
||||
rowsToPersist.push({
|
||||
roomId,
|
||||
eventId: event.getId() as string,
|
||||
ts,
|
||||
sender,
|
||||
body: text.body,
|
||||
...(text.formattedBody ? { formattedBody: text.formattedBody } : {}),
|
||||
...(text.pollText ? { pollText: text.pollText } : {}),
|
||||
});
|
||||
if (cacheEnabled && event.getId()) {
|
||||
// An edit (`m.replace`) event's own body is just a "* new text"
|
||||
// fallback. Indexing it under its own event id would leave two
|
||||
// separate matching rows (the stale pre-edit text and the edit
|
||||
// fallback) searchable forever. Instead, upsert the *original*
|
||||
// event's row with the edit's `m.new_content` (Gitea #14).
|
||||
const editTargetId =
|
||||
event.getRelation()?.rel_type === RelationType.Replace
|
||||
? event.getRelation()?.event_id
|
||||
: undefined;
|
||||
if (editTargetId) {
|
||||
const newContent = (event.getContent()['m.new_content'] ?? {}) as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
const editedText = extractTextFromContent(EventType.RoomMessage, newContent);
|
||||
if (editedText) {
|
||||
rowsToPersist.push({
|
||||
roomId,
|
||||
eventId: editTargetId,
|
||||
ts: room.findEventById(editTargetId)?.getTs() ?? ts,
|
||||
sender,
|
||||
body: editedText.body,
|
||||
...(editedText.formattedBody ? { formattedBody: editedText.formattedBody } : {}),
|
||||
});
|
||||
}
|
||||
} else if (text) {
|
||||
rowsToPersist.push({
|
||||
roomId,
|
||||
eventId: event.getId() as string,
|
||||
ts,
|
||||
sender,
|
||||
body: text.body,
|
||||
...(text.formattedBody ? { formattedBody: text.formattedBody } : {}),
|
||||
...(text.pollText ? { pollText: text.pollText } : {}),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (senderSet && !senderSet.has(sender)) continue;
|
||||
@@ -239,7 +284,12 @@ export const useLocalMessageSearch = () => {
|
||||
if (senderSet && !senderSet.has(row.sender)) return;
|
||||
if (!inRange(row.ts)) return;
|
||||
if (!senderOnlyMode && !rowMatchesTerm(row, termLower)) return;
|
||||
cachedItems.push(rowToResultItem(row));
|
||||
// The cache-delete listener (searchCacheInvalidation.ts) removes a
|
||||
// row on redaction asynchronously; if the event is still around
|
||||
// locally we can check for certain and must not surface stale
|
||||
// plaintext in the meantime (Gitea #14).
|
||||
const localEvent = room.findEventById(row.eventId);
|
||||
cachedItems.push(rowToResultItem(row, localEvent?.isRedacted()));
|
||||
});
|
||||
|
||||
const items = mergeSearchResults(memoryItems, cachedItems);
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
saveRoomIndex,
|
||||
clearRoom,
|
||||
clearAll,
|
||||
deleteRow,
|
||||
deleteSearchCacheDatabase,
|
||||
SearchCacheRow,
|
||||
} from './searchCache';
|
||||
@@ -128,6 +129,31 @@ test('searchCache IDB round-trip', { skip: !hasIdb }, async () => {
|
||||
await deleteSearchCacheDatabase();
|
||||
});
|
||||
|
||||
test('deleteRow: removes only the targeted [roomId, eventId] row', { skip: !hasIdb }, async () => {
|
||||
await clearAll();
|
||||
const rows: SearchCacheRow[] = [
|
||||
{ roomId: '!r1', eventId: '$1', ts: 100, sender: '@a', body: 'hello' },
|
||||
{ roomId: '!r1', eventId: '$2', ts: 200, sender: '@b', body: 'world' },
|
||||
{ roomId: '!r2', eventId: '$1', ts: 300, sender: '@a', body: 'other room, same id' },
|
||||
];
|
||||
await putRows(rows);
|
||||
|
||||
await deleteRow('!r1', '$1');
|
||||
|
||||
const r1 = await queryRoom('!r1');
|
||||
assert.deepEqual(
|
||||
r1.map((x) => x.eventId),
|
||||
['$2'],
|
||||
);
|
||||
// A same-eventId row in a different room is untouched (composite key).
|
||||
assert.equal((await queryRoom('!r2')).length, 1);
|
||||
|
||||
// Deleting a row that doesn't exist is a silent no-op.
|
||||
await assert.doesNotReject(deleteRow('!r1', '$does-not-exist'));
|
||||
|
||||
await deleteSearchCacheDatabase();
|
||||
});
|
||||
|
||||
test('resilient helpers never throw when IDB is unavailable', { skip: hasIdb }, async () => {
|
||||
// In this environment IndexedDB is absent; every call must degrade to a
|
||||
// cache-miss rather than throwing.
|
||||
@@ -138,6 +164,7 @@ test('resilient helpers never throw when IDB is unavailable', { skip: hasIdb },
|
||||
assert.equal(await getCoverage('!r'), null);
|
||||
await assert.doesNotReject(saveRoomIndex('!r', []));
|
||||
await assert.doesNotReject(clearRoom('!r'));
|
||||
await assert.doesNotReject(deleteRow('!r', '$1'));
|
||||
await assert.doesNotReject(clearAll());
|
||||
await assert.doesNotReject(deleteSearchCacheDatabase());
|
||||
});
|
||||
|
||||
@@ -288,6 +288,23 @@ export const mergeSearchResults = <
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Delete a single cached row, e.g. because its event was redacted. Gitea #14:
|
||||
* without this, a redaction only ever removed the in-memory hit — the
|
||||
* decrypted plaintext stayed in IndexedDB forever.
|
||||
*/
|
||||
export const deleteRow = async (roomId: string, eventId: string): Promise<void> => {
|
||||
const db = await openDb();
|
||||
if (!db) return;
|
||||
try {
|
||||
const tx = db.transaction(MESSAGES_STORE, 'readwrite');
|
||||
tx.objectStore(MESSAGES_STORE).delete([roomId, eventId]);
|
||||
await awaitTx(tx);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
};
|
||||
|
||||
export const clearRoom = async (roomId: string): Promise<void> => {
|
||||
const db = await openDb();
|
||||
if (!db) return;
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
import { useEffect } from 'react';
|
||||
import { useAtomValue } from 'jotai';
|
||||
import { MatrixEvent, Room, RoomEvent } from 'matrix-js-sdk';
|
||||
import { useMatrixClient } from '../hooks/useMatrixClient';
|
||||
import { searchCacheEnabledAtom } from '../state/searchCacheEnabled';
|
||||
import { clearRoom, deleteRow } from './searchCache';
|
||||
|
||||
/**
|
||||
* Gitea #14 — the persistent search cache (`searchCache.ts`) had no
|
||||
* invalidation path other than logout or the manual "Clear cached index"
|
||||
* button, so redacted messages and rooms the user left kept their decrypted
|
||||
* plaintext searchable on disk indefinitely.
|
||||
*
|
||||
* While the cache is enabled, listen client-wide for redactions (delete the
|
||||
* redacted row) and for leaving/being banned from a room (wipe the room's
|
||||
* cached rows via the existing `clearRoom`).
|
||||
*/
|
||||
export const useSearchCacheInvalidation = (): void => {
|
||||
const mx = useMatrixClient();
|
||||
const cacheEnabled = useAtomValue(searchCacheEnabledAtom);
|
||||
|
||||
useEffect(() => {
|
||||
if (!cacheEnabled) return undefined;
|
||||
|
||||
const onRedaction = (event: MatrixEvent, room: Room) => {
|
||||
const redactedEventId = event.getAssociatedId();
|
||||
if (redactedEventId) deleteRow(room.roomId, redactedEventId);
|
||||
};
|
||||
|
||||
const onMyMembership = (room: Room, membership: string) => {
|
||||
if (membership === 'leave' || membership === 'ban') {
|
||||
clearRoom(room.roomId);
|
||||
}
|
||||
};
|
||||
|
||||
mx.on(RoomEvent.Redaction, onRedaction);
|
||||
mx.on(RoomEvent.MyMembership, onMyMembership);
|
||||
return () => {
|
||||
mx.off(RoomEvent.Redaction, onRedaction);
|
||||
mx.off(RoomEvent.MyMembership, onMyMembership);
|
||||
};
|
||||
}, [mx, cacheEnabled]);
|
||||
};
|
||||
Reference in New Issue
Block a user