Skip to content

fix(ui): Decrypt TimelineEventKind::UnableToDecrypt coming from the event cache #4794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Mar 12, 2025

The fix is 3 LOC, the test is 345 LOC, oh yeah.

So, currently, it is possible to store an TimelineEventKind::UnableToDecrypt (UTD) event inside the event cache. When received by the Timeline, prior this patch, such event wasn't decrypted. The fix is actually quite small: everything that comes from the cache (EventsOrigin::Cache) and that contains a UTD, triggers a redecryption. Hoping that the keys are in the OlmMachine of course.

This patch tests two situations where it can happen: when the UTD is part of the initial items, and when the UTD is part of a pagination. The former was already working, but not tested via the event cache as far as I know, and the latter wasn't tested at all (because it wasn't implemented).


Copy link

codecov bot commented Mar 12, 2025

Codecov Report

Attention: Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.38%. Comparing base (5a22944) to head (b86b497).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk-ui/src/timeline/builder.rs 66.66% 2 Missing ⚠️
crates/matrix-sdk-ui/src/timeline/mod.rs 50.00% 1 Missing ⚠️
crates/matrix-sdk-ui/src/timeline/to_device.rs 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4794      +/-   ##
==========================================
+ Coverage   86.36%   86.38%   +0.02%     
==========================================
  Files         291      291              
  Lines       34379    34374       -5     
==========================================
+ Hits        29691    29695       +4     
+ Misses       4688     4679       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Hywan added 2 commits March 14, 2025 10:53
…needs a `&Room`.

This patch removes the room `&Room` argument of
`TimelineController::retry_event_decryption`. The `TimelineController`
already has the room with its `room(&self) -> &Room` method. This method
was always used to fetch the room, let's expect `retry_event_decryption`
to do that by itself.

It also prevents passing the “wrong” room. This is more robust this way.
This patch fixes a bug where events coming from the event cache might be
encrypted, see matrix-org#4762
to learn more.

This patch updates the `room_event_cache_updates_task` to call
`TimelineController::retry_event_decryption` if the origin is `Cache`.
@Hywan Hywan force-pushed the fix-ui-timeline-decrypt-utd-from-event-cache branch from 3bada45 to ddc9345 Compare March 14, 2025 09:53
@Hywan Hywan marked this pull request as ready for review March 14, 2025 12:31
@Hywan Hywan requested a review from a team as a code owner March 14, 2025 12:31
@Hywan Hywan requested review from andybalaam and poljar and removed request for a team and andybalaam March 14, 2025 12:31
@Hywan Hywan changed the title fix(ui): Decrypt m.room.encrypted coming from the event cache fix(ui): Decrypt TimelineEventKind::UnableToDecrypt coming from the event cache Mar 14, 2025
Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

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

Looks good, I left some philosophical musings and nits.

This patch adds two tests, ensuring UTD stored in the event cache are
decrypted, whether they come from the initial items or paginated items.
@Hywan Hywan force-pushed the fix-ui-timeline-decrypt-utd-from-event-cache branch from 75043d4 to b86b497 Compare March 14, 2025 13:00
@Hywan Hywan merged commit 17c6ad6 into matrix-org:main Mar 14, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants