Skip to content

Set MSC4293 flag when autoredacting users#612

Merged
H-Shay merged 2 commits into
mainfrom
travis/redact-on-ban
Jun 12, 2025
Merged

Set MSC4293 flag when autoredacting users#612
H-Shay merged 2 commits into
mainfrom
travis/redact-on-ban

Conversation

@turt2live

Copy link
Copy Markdown
Member

@turt2live turt2live marked this pull request as ready for review June 9, 2025 21:38
@turt2live turt2live requested a review from a team as a code owner June 9, 2025 21:38

@H-Shay H-Shay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@H-Shay H-Shay merged commit 41a0ca2 into main Jun 12, 2025
5 checks passed
@H-Shay H-Shay deleted the travis/redact-on-ban branch June 12, 2025 23:24
Comment thread src/ProtectedRoomsSet.ts
Comment on lines +490 to 498
// Use MSC4293 and still fall back later
// See https://github.com/matrix-org/matrix-spec-proposals/pull/4293
await this.client.sendStateEvent(roomId, "m.room.member", member.userId, {
"membership": "ban",
"org.matrix.msc4293.redact_events": true,
});

// Fallback
this.redactUser(member.userId, roomId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a risk here if Mjolnir fails to redact events in the fallback. This can happen if there are more events in the timeline than the pagination limit, or if a call to /redact fails. Trying to workaround by using !mjolnir redact will no longer work in this room because Mjolnir will no longer receive unredacted events from /messages due to the behavior of MSC4293.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Receiving redacted events might be fine though? The logic could probably inspect the reason if it wanted too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, It could be fine, but only because mjolnir doesn't exclude the redacted events from being called with /redact again

mjolnir/src/utils.ts

Lines 102 to 121 in 41a0ca2

await getMessagesByUserIn(client, userIdOrGlob, targetRoomId, limit, async (eventsToRedact) => {
for (const targetEvent of eventsToRedact) {
await managementRoom.logMessage(
LogLevel.DEBUG,
"utils#redactUserMessagesIn",
`Redacting ${targetEvent["event_id"]} in ${targetRoomId}`,
targetRoomId,
);
if (!noop) {
await client.redactEvent(targetRoomId, targetEvent["event_id"]);
} else {
await managementRoom.logMessage(
LogLevel.WARN,
"utils#redactUserMessagesIn",
`Tried to redact ${targetEvent["event_id"]} in ${targetRoomId} but Mjolnir is running in no-op mode`,
targetRoomId,
);
}
}
});

Comment thread src/ProtectedRoomsSet.ts
// See https://github.com/matrix-org/matrix-spec-proposals/pull/4293
await this.client.sendStateEvent(roomId, "m.room.member", member.userId, {
"membership": "ban",
"org.matrix.msc4293.redact_events": true,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was reason intentionally omitted here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants