From 12fdfc96a38ada9ff0fd7e816235ef4105eaa403 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sun, 8 Feb 2026 09:02:10 -0300 Subject: [PATCH] fix: Don't resend webxdc status updates in OutBroadcast-s (#7679) We don't remember which status updates are "initial" (sent initially by the broadcast owner) and which were sent by subscribers, so don't resend any of them to avoid accidental sharing of confidential data. --- src/chat.rs | 2 +- src/constants.rs | 2 ++ src/message.rs | 6 ++++ src/mimefactory.rs | 17 ++++++------ src/webxdc/webxdc_tests.rs | 56 ++++++++++++++++++++++++++++++++------ 5 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 6fe43637e..c0455a589 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4682,7 +4682,7 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { // note(treefit): only matters if it is the last message in chat (but probably to expensive to check, debounce also solves it) chatlist_events::emit_chatlist_item_changed(context, msg.chat_id); - if msg.viewtype == Viewtype::Webxdc { + if msg.viewtype == Viewtype::Webxdc && msg.chat_typ != Chattype::OutBroadcast { let conn_fn = |conn: &mut rusqlite::Connection| { let range = conn.query_row( "SELECT IFNULL(min(id), 1), IFNULL(max(id), 0) \ diff --git a/src/constants.rs b/src/constants.rs index 28eb4c0fe..2180fc7b8 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -107,12 +107,14 @@ pub const DC_CHAT_ID_LAST_SPECIAL: ChatId = ChatId::new(9); IntoStaticStr, Serialize, Deserialize, + Default, )] #[repr(u32)] pub enum Chattype { /// A 1:1 chat, i.e. a normal chat with a single contact. /// /// Created by [`ChatId::create_for_contact`]. + #[default] Single = 100, /// Group chat. diff --git a/src/message.rs b/src/message.rs index c34a43093..118b90043 100644 --- a/src/message.rs +++ b/src/message.rs @@ -453,6 +453,7 @@ pub struct Message { pub(crate) is_dc_message: MessengerMessage, pub(crate) original_msg_id: MsgId, pub(crate) mime_modified: bool, + pub(crate) chat_typ: Chattype, pub(crate) chat_visibility: ChatVisibility, pub(crate) chat_blocked: Blocked, pub(crate) location_id: u32, @@ -527,6 +528,7 @@ impl Message { m.param AS param, m.hidden AS hidden, m.location_id AS location, + c.type AS chat_typ, c.archived AS visibility, c.blocked AS blocked FROM msgs m @@ -586,6 +588,10 @@ impl Message { param: row.get::<_, String>("param")?.parse().unwrap_or_default(), hidden: row.get("hidden")?, location_id: row.get("location")?, + // This is safe: see `ChatId::delete_ex()`, `None` chat type can't happen. + chat_typ: row + .get::<_, Option<_>>("chat_typ")? + .unwrap_or(Chattype::Single), chat_visibility: row.get::<_, Option<_>>("visibility")?.unwrap_or_default(), chat_blocked: row .get::<_, Option>("blocked")? diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 986ca2f04..c271e4349 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2037,14 +2037,15 @@ impl MimeFactory { HeaderDef::IrohGossipTopic.get_headername(), mail_builder::headers::raw::Raw::new(topic).into(), )); - if let (Some(json), _) = context - .render_webxdc_status_update_object( - msg.id, - StatusUpdateSerial::MIN, - StatusUpdateSerial::MAX, - None, - ) - .await? + if msg.chat_typ != Chattype::OutBroadcast + && let (Some(json), _) = context + .render_webxdc_status_update_object( + msg.id, + StatusUpdateSerial::MIN, + StatusUpdateSerial::MAX, + None, + ) + .await? { parts.push(context.build_status_update_part(&json)); } diff --git a/src/webxdc/webxdc_tests.rs b/src/webxdc/webxdc_tests.rs index 00ff207bc..fffd0df84 100644 --- a/src/webxdc/webxdc_tests.rs +++ b/src/webxdc/webxdc_tests.rs @@ -1600,33 +1600,49 @@ async fn test_in_broadcast_send_status_update() -> Result<()> { let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; - let alice_instance = send_webxdc_instance(alice, alice_chat_id).await?; + let mut alice_instance = create_webxdc_instance( + alice, + "minimal.xdc", + include_bytes!("../../test-data/webxdc/minimal.xdc"), + )?; + alice_chat_id + .set_draft(alice, Some(&mut alice_instance)) + .await?; + alice_instance = alice_chat_id.get_draft(alice).await?.unwrap(); + alice + .send_webxdc_status_update(alice_instance.id, r#"{"payload":41}"#) + .await?; + send_msg(alice, alice_chat_id, &mut alice_instance).await?; let sent_msg = alice.pop_sent_msg().await; let bob_instance = bob.recv_msg(&sent_msg).await; assert_eq!(bob_instance.chat_id, bob_chat_id); let provider = BackupProvider::prepare(bob).await?; - let bob1 = &tcm.unconfigured().await; - get_backup(bob1, provider.qr()).await?; + let bob2 = &tcm.unconfigured().await; + get_backup(bob2, provider.qr()).await?; bob.send_webxdc_status_update(bob_instance.id, r#"{"payload":42}"#) .await?; bob.flush_status_updates().await?; - let sent_msg = bob.pop_sent_msg().await; + // Don't wait to make sure that status updates are sent immediately, otherwise the check for + // Alice below isn't reliable. + let sent_msg = bob.pop_sent_msg_opt(Duration::ZERO).await.unwrap(); alice.recv_msg_trash(&sent_msg).await; assert_eq!( alice .get_webxdc_status_updates(alice_instance.id, StatusUpdateSerial(0)) .await?, - r#"[{"payload":42,"serial":1,"max_serial":1}]"# + r#"[{"payload":41,"serial":1,"max_serial":2}, +{"payload":42,"serial":2,"max_serial":2}]"# ); - bob1.recv_msg_trash(&sent_msg).await; + bob2.recv_msg_trash(&sent_msg).await; assert_eq!( - bob1.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0)) + bob2.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0)) .await?, - r#"[{"payload":42,"serial":1,"max_serial":1}]"# + r#"[{"payload":41,"serial":1,"max_serial":2}, +{"payload":42,"serial":2,"max_serial":2}]"# ); // Non-subscriber's status updates are rejected. @@ -1635,7 +1651,29 @@ async fn test_in_broadcast_send_status_update() -> Result<()> { alice.pop_sent_msg().await; let status = helper_send_receive_status_update(bob, alice, &bob_instance, &alice_instance).await?; - assert_eq!(status, r#"[{"payload":42,"serial":1,"max_serial":1}]"#); + assert_eq!( + status, + r#"[{"payload":41,"serial":1,"max_serial":2}, +{"payload":42,"serial":2,"max_serial":2}]"# + ); + + // Subscribers' status updates are confidential and shalln't be re-sent. So initial status + // updates aren't re-sent too. + let fiona = &tcm.fiona().await; + let fiona_chat_id = tcm.exec_securejoin_qr(fiona, alice, &qr).await; + resend_msgs(alice, &[alice_instance.id]).await?; + let sent1 = alice.pop_sent_msg().await; + alice.flush_status_updates().await?; + assert!(alice.pop_sent_msg_opt(Duration::ZERO).await.is_none()); + let fiona_instance = fiona.recv_msg(&sent1).await; + assert_eq!(fiona_instance.chat_id, fiona_chat_id); + assert_eq!(fiona_instance.chat_typ, Chattype::InBroadcast); + assert_eq!( + fiona + .get_webxdc_status_updates(fiona_instance.id, StatusUpdateSerial(0)) + .await?, + r#"[]"# + ); Ok(()) }