matrix-org / matrix-org/matrix-js-sdk

MSC1763 local retention is bypassed for events paginated into filtered timeline sets

Open
#5,481 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

T-Defect
Dominant language
TypeScript
Stars
2.2k
Forks
704
Avg merge
1d 20h
Merged PRs (30d)
40

Description

### Description

When `unstableMSC1763Retention` is enabled and a room has an effective
`m.room.retention` policy with `max_lifetime`, expired events can still
become visible after backward pagination.

This can be reproduced in Element Web with `feature_retention` enabled:
messages older than `max_lifetime` disappear initially, but scrolling
far enough backwards can load and display them again.

### Suspected cause

There appear to be two related paths:

1. `Room.addEventsToTimeline()` checks
`this.retention?.shouldEventBeRetained(event)`, but pagination code
calls `EventTimelineSet.addEventsToTimeline()` directly.

2. `RoomRetentionPolicy.processTimeline()` only scans
`room.getLiveTimeline()`. Events inserted into filtered or detached
timeline sets are therefore not found by the cleanup pass.

### Minimal regression test

Tested against matrix-js-sdk develop commit:

`2a625eb8f611a701867ac47f3e30345cfbb13f00`

```ts
it("does not expose expired events paginated into a filtered timeline", async () => {
getCachedMock.mockReturnValue({
policies: {
[ROOM_ID]: { max_lifetime: ONE_DAY_MS },
},
});
await applyPolicy();

const filter = new Filter(USER_ID, "test_filter");
filter.setDefinition({});

const timelineSet = room.getOrCreateFilteredTimelineSet(filter, {
prepopulateTimeline: false,
useSyncEvents: false,
});

const expiredEvent = makeMessageEvent(
Date.now() - 2 * ONE_DAY_MS,
);

timelineSet.addEventsToTimeline(
[expiredEvent],
true,
false,
timelineSet.getLiveTimeline(),
null,
);

await vi.advanceTimersByTimeAsync(201);

expect(
timelineSet.getLiveTimeline().getEvents(),
).toHaveLength(0);
});
```

Result:

```text
Expected: 0 events
Received: 1 event
```

All 31 existing room-retention tests passed; only this new pagination
regression test failed.

Runtime logging also reported:

```text
Running processTimeline
Found no expired events
```

### Expected behavior

Expired events should not be inserted into any visible timeline set, or
they should be synthetically redacted/removed immediately after
pagination.

### Actual behavior

Expired events inserted through historical pagination remain in a
filtered timeline set and can be displayed.

### Affected consumer

Reproduced in Element Web 1.12.25 with `feature_retention` enabled and a
room policy using `max_lifetime`.

### Possible direction

Either retention should be enforced at the `EventTimelineSet` insertion
boundary, pagination should route through the retention-aware Room API,
or the cleanup process should cover all relevant room timeline sets.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Room.addEventsToTimeline(), EventTimelineSet.addEventsToTimeline(), and RoomRetentionPolicy.processTimeline(), then run the supplied filtered-timeline regression test alongside the existing room-retention tests. Trace how paginated events enter filtered or detached timeline sets and how cleanup discovers them. Done means expired events are not visible after pagination and the regression test passes without breaking the existing retention suite.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.