From 8b2322f751a0ae1bcb839bbb4b9dfee5510a12d6 Mon Sep 17 00:00:00 2001 From: Lotus CI Date: Fri, 16 Oct 2026 21:37:16 -0400 Subject: [PATCH] fix(retention): a tiny or brand-new retention value can't wipe a room (#298) m.room.retention can be set to anything by anyone with state power. A fat-fingered `max_lifetime: 1000` blanked the timeline for every Lotus user and, for members with "Enforce Message Retention" on, the sweeper redacted all their messages within a minute. The RETENTION_MIN_MS floor existed but was never applied. - effectiveRetentionMs(): anything under 10 min counts as off; used by getRoomRetentionMs (sweeper) and the timeline's hiding. - The sweeper never deletes under a policy less than 24 h old, so a mistake or a malicious change can be reverted before anything is lost. (Local hiding of expired messages still applies at once.) Verified headless (the issue's repro, enforcement on): after max_lifetime 1000, all 4 messages stay visible and none are redacted after 45 s (before: timeline blank, all 4 redacted). Unit tests for the floor and the grace window. Fixes #298 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017PCb59uY3UKqDqt7B6hzd5 --- src/app/features/room/RoomTimeline.tsx | 7 +++-- src/app/pages/client/ClientNonUIFeatures.tsx | 10 ++++++- src/app/utils/retention.test.ts | 28 ++++++++++++++++++- src/app/utils/retention.ts | 29 ++++++++++++++++++-- 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index fd69479c4..2500feda6 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -121,7 +121,7 @@ import { ThreadSummary } from './thread/ThreadSummary'; import { usePowerLevelsContext } from '../../hooks/usePowerLevels'; import { GetContentCallback, MessageEvent, StateEvent } from '../../../types/matrix/room'; import { useStateEvent } from '../../hooks/useStateEvent'; -import { RetentionContent, isExpired } from '../../utils/retention'; +import { RetentionContent, effectiveRetentionMs, isExpired } from '../../utils/retention'; import { useKeyDown } from '../../hooks/useKeyDown'; import { useDocumentFocusChange } from '../../hooks/useDocumentFocusChange'; import { RenderMessageContent } from '../../components/RenderMessageContent'; @@ -494,7 +494,10 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli // timeline (unless "show hidden events" is on). Reactive so a policy change // re-renders. `undefined` = no policy. const retentionEvent = useStateEvent(room, StateEvent.RoomRetention); - const retentionMs = retentionEvent?.getContent().max_lifetime; + // [Gitea #298] Same floor as everywhere else: a tiny value must not blank the room. + const retentionMs = effectiveRetentionMs( + retentionEvent?.getContent().max_lifetime, + ); const [showDeveloperTools] = useSetting(settingsAtom, 'developerTools'); const [lotusTerminal] = useSetting(settingsAtom, 'lotusTerminal'); diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index 84e3e9f8f..6bf40cbe2 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -68,7 +68,12 @@ import { setPresenceWithRetry } from '../../utils/presenceWrite'; import { useDeepLinkNavigate } from '../../hooks/useDeepLinkNavigate'; import { dismissToastAtom, toastQueueAtom } from '../../state/toast'; import { useReminders } from '../../hooks/useReminders'; -import { getRoomRetentionMs, isExpired } from '../../utils/retention'; +import { + getRoomRetentionMs, + getRoomRetentionSetAt, + isExpired, + retentionDeletesAllowed, +} from '../../utils/retention'; import { useTauriUpdateProgress, useTauriUpdater } from '../../hooks/useTauriUpdater'; import { settingsRequestAtom } from '../../state/settingsRequest'; import { isNetworkUpdateError } from '../../utils/updateErrors'; @@ -910,6 +915,9 @@ function RetentionSweeper() { mx.getRooms().forEach((room) => { const maxLifetime = getRoomRetentionMs(room); if (!maxLifetime) return; + // [Gitea #298] Never delete under a policy that is less than a day old. + const setAt = getRoomRetentionSetAt(room); + if (setAt === undefined || !retentionDeletesAllowed(setAt, now)) return; room .getLiveTimeline() .getEvents() diff --git a/src/app/utils/retention.test.ts b/src/app/utils/retention.test.ts index 1155eb7df..aeac666df 100644 --- a/src/app/utils/retention.test.ts +++ b/src/app/utils/retention.test.ts @@ -1,6 +1,13 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { isExpired, RETENTION_PRESETS, RETENTION_MIN_MS } from './retention'; +import { + effectiveRetentionMs, + isExpired, + RETENTION_DELETE_GRACE_MS, + RETENTION_MIN_MS, + RETENTION_PRESETS, + retentionDeletesAllowed, +} from './retention'; // MSC1763 retention: `isExpired` decides whether a message is past the room's // retention window. It must be strict (> window, not >=) and a disabled policy @@ -40,3 +47,22 @@ test('presets: Off is 0 and the rest are strictly increasing, all >= the floor', 'all presets above the floor', ); }); + +test('effectiveRetentionMs: tiny or invalid windows count as off (#298)', () => { + assert.equal(effectiveRetentionMs(1000), undefined, '1 s is a typo, not a policy'); + assert.equal(effectiveRetentionMs(RETENTION_MIN_MS - 1), undefined); + assert.equal(effectiveRetentionMs(RETENTION_MIN_MS), RETENTION_MIN_MS); + assert.equal(effectiveRetentionMs(86_400_000), 86_400_000); + assert.equal(effectiveRetentionMs(0), undefined); + assert.equal(effectiveRetentionMs(-5), undefined); + assert.equal(effectiveRetentionMs('86400000'), undefined); + assert.equal(effectiveRetentionMs(Number.NaN), undefined); +}); + +test('retentionDeletesAllowed: no deletions in the first 24 h of a policy (#298)', () => { + const now = 1_000 * HOUR; + assert.equal(retentionDeletesAllowed(now - 2 * HOUR, now), false); + assert.equal(retentionDeletesAllowed(now - RETENTION_DELETE_GRACE_MS + 1, now), false); + assert.equal(retentionDeletesAllowed(now - RETENTION_DELETE_GRACE_MS, now), true); + assert.equal(retentionDeletesAllowed(now - 72 * HOUR, now), true); +}); diff --git a/src/app/utils/retention.ts b/src/app/utils/retention.ts index 4573dcc12..6c7dcd68c 100644 --- a/src/app/utils/retention.ts +++ b/src/app/utils/retention.ts @@ -20,13 +20,38 @@ export const RETENTION_PRESETS: RetentionPreset[] = [ { label: '1 Month', ms: 30 * DAY_MS }, ]; +/** + * [Gitea #298] A usable retention window, or `undefined`. Anything under + * RETENTION_MIN_MS is treated as off: a fat-fingered `max_lifetime: 1000` (set + * from any client by anyone with state power) used to blank every timeline and + * get opted-in users' messages redacted within a minute. + */ +export const effectiveRetentionMs = (maxLifetime: unknown): number | undefined => + typeof maxLifetime === 'number' && Number.isFinite(maxLifetime) && maxLifetime >= RETENTION_MIN_MS + ? maxLifetime + : undefined; + /** The room's active retention window in ms, or `undefined` when unset/disabled. */ export const getRoomRetentionMs = (room: Room): number | undefined => { const event = room.currentState.getStateEvents(StateEvent.RoomRetention, ''); - const ms = event?.getContent()?.max_lifetime; - return typeof ms === 'number' && ms > 0 ? ms : undefined; + return effectiveRetentionMs(event?.getContent()?.max_lifetime); }; +/** + * [Gitea #298] Deletion grace: the auto-delete sweeper ignores a retention + * policy for this long after it was set, so a mistaken or malicious change can + * be reverted before anything is permanently redacted. (Hiding expired + * messages locally is not destructive and applies at once.) + */ +export const RETENTION_DELETE_GRACE_MS = DAY_MS; + +export const retentionDeletesAllowed = (policySetAtMs: number, nowMs: number): boolean => + nowMs - policySetAtMs >= RETENTION_DELETE_GRACE_MS; + +/** When the room's current retention policy was set (its state event's ts). */ +export const getRoomRetentionSetAt = (room: Room): number | undefined => + room.currentState.getStateEvents(StateEvent.RoomRetention, '')?.getTs(); + /** True when an event at `tsMs` has passed the `maxLifetimeMs` retention window. */ export const isExpired = (tsMs: number, maxLifetimeMs: number, nowMs: number): boolean => maxLifetimeMs > 0 && nowMs - tsMs > maxLifetimeMs;