Merge ab5161226ba522d1e9ca0f6931fd1b3244b4242c into 0d38dd80b75f2be3e4320caa5491e44d50ad8436

bin/chat-cli/src/main.rs

@@ -119,6 +119,7 @@ fn run<T: Transport>(transport: T, cli: &Cli) -> Result<()> {
             let (client, events) = ChatClientBuilder::new()
                 .transport(transport)
                 .storage_config(storage)
+                .map_err(|e| anyhow::anyhow!("{e:?}"))?
                 .registration(registry)
                 .build()
                 .map_err(|e| anyhow::anyhow!("{e:?}"))
@@ -129,6 +130,7 @@ fn run<T: Transport>(transport: T, cli: &Cli) -> Result<()> {
             let (client, events) = ChatClientBuilder::new()
                 .transport(transport)
                 .storage_config(storage)
+                .map_err(|e| anyhow::anyhow!("{e:?}"))?
                 .build()
                 .map_err(|e| anyhow::anyhow!("{e:?}"))
                 .context("failed to open chat client")?;
@@ -197,6 +199,7 @@ fn run_logos_delivery(cli: Cli) -> Result<()> {
                         path: db_str,
                         key: "chat-cli".to_string(),
                     })
+                    .map_err(|e| anyhow::anyhow!("{e:?}"))?
                     .transport(delivery)
                     .build()
                     .map_err(|e| anyhow::anyhow!("{e:?}"))

core/conversations/src/conversation/group_v1.rs

@@ -254,7 +254,10 @@ impl<S: ExternalServices> Convo<S> for GroupV1Convo {
         let msg_hash = blake2b_hex::<hash_size::MessageId>(&[bytes.as_ref()]);
         if self.outbound_msgs.contains(&msg_hash) {
             debug!("Dropping message, sent from self");
-            return Ok(ConvoOutcome::empty(self.convo_id.to_string()));
+            return Ok(ConvoOutcome::empty(
+                self.convo_id.to_string(),
+                crate::ConversationClass::Group,
+            ));
         }
 
         let mls_message: MlsMessageIn =
@@ -266,7 +269,10 @@ impl<S: ExternalServices> Convo<S> for GroupV1Convo {
 
         if protocol_message.epoch() < self.mls_group.epoch() {
             // TODO: (P1) Add logging for messages arriving from past epoch.
-            return Ok(ConvoOutcome::empty(self.id().to_string()));
+            return Ok(ConvoOutcome::empty(
+                self.id().to_string(),
+                crate::ConversationClass::Group,
+            ));
         }
 
         let processed = self
@@ -299,6 +305,7 @@ impl<S: ExternalServices> Convo<S> for GroupV1Convo {
         Ok(ConvoOutcome {
             convo_id: self.id().to_string(),
             content,
+            class: crate::ConversationClass::Group,
         })
     }
 

core/conversations/src/conversation/group_v2.rs

@@ -239,7 +239,12 @@ where
         let frame = GroupV2Frame::decode(bytes.as_ref()).map_err(ChatError::generic)?;
         let inner = match frame.payload {
             Some(GroupV2Payload::DeMlsWrapper(b)) => b.to_vec(),
-            _ => return Ok(ConvoOutcome::empty(self.convo_id.clone())),
+            _ => {
+                return Ok(ConvoOutcome::empty(
+                    self.convo_id.clone(),
+                    crate::ConversationClass::Group,
+                ));
+            }
         };
 
         self.conversation.process_inbound(
@@ -256,7 +261,10 @@ where
             Some(o) => Ok(o),
             None => {
                 warn!("returning None as ConvoOutcome");
-                Ok(ConvoOutcome::empty(self.convo_id.to_string()))
+                Ok(ConvoOutcome::empty(
+                    self.convo_id.to_string(),
+                    crate::ConversationClass::Group,
+                ))
             }
         }
     }
@@ -387,6 +395,7 @@ impl GroupV2Convo {
                     bytes: cm.message.clone(),
                     encoded_credential: cm.sender.clone(),
                 }),
+                class: crate::ConversationClass::Group,
             }),
             _ => None,
         })

core/conversations/src/conversation/privatev1.rs

@@ -276,6 +276,7 @@ impl<S: ExternalServices> Convo<S> for PrivateV1Convo {
         Ok(ConvoOutcome {
             convo_id: self.id().to_string(),
             content,
+            class: crate::ConversationClass::Private,
         })
     }
 

core/conversations/src/outcomes.rs

@@ -22,13 +22,18 @@ pub struct Content {
 pub struct ConvoOutcome {
     pub convo_id: ConversationId,
     pub content: Option<Content>,
+    /// Class of the conversation this outcome belongs to. Surfaced so a
+    /// consumer can tell an anonymous PrivateV1 message (no sender credential
+    /// by design) from a group message that is missing one.
+    pub class: ConversationClass,
 }
 
 impl ConvoOutcome {
-    pub fn empty(convo_id: ConversationId) -> Self {
+    pub fn empty(convo_id: ConversationId, class: ConversationClass) -> Self {
         Self {
             convo_id,
             content: None,
+            class,
         }
     }
 }

crates/client/src/builder.rs

@@ -74,17 +74,18 @@ impl<I, T, R, S> ChatClientBuilder<I, T, R, S> {
         }
     }
 
-    pub fn storage_config(self, config: StorageConfig) -> ChatClientBuilder<I, T, R, ChatStorage> {
+    pub fn storage_config(
-        let storage = ChatStorage::new(config)
+        self,
-            .map_err(ChatError::from)
+        config: StorageConfig,
-            .expect("Storage config file should be valid");
+    ) -> Result<ChatClientBuilder<I, T, R, ChatStorage>, ChatError> {
+        let storage = ChatStorage::new(config).map_err(ChatError::from)?;
 
-        ChatClientBuilder {
+        Ok(ChatClientBuilder {
             ident: self.ident,
             transport: self.transport,
             registration: self.registration,
             storage,
-        }
+        })
     }
 }
 

crates/client/src/client.rs

@@ -5,8 +5,9 @@ use components::{ThreadedWakeupService, WakeupEvent};
 use crossbeam_channel::{Receiver, Sender, select};
 use crypto::Ed25519VerifyingKey;
 use libchat::{
-    AccountDirectory, ConversationId, ConvoOutcome, Core, DeliveryService, IdentId, IdentIdRef,
+    AccountDirectory, ConversationClass, ConversationId, ConvoOutcome, Core, DeliveryService,
-    IdentityProvider, InboxOutcome, Introduction, PayloadOutcome, RegistrationService,
+    IdentId, IdentIdRef, IdentityProvider, InboxOutcome, Introduction, PayloadOutcome,
+    RegistrationService,
 };
 use parking_lot::Mutex;
 use storage::ChatStore;
@@ -260,8 +261,9 @@ fn account_key_from_hex(addr: &str) -> Option<Ed25519VerifyingKey> {
 /// Why a message's sender could not be accepted, so the message is dropped.
 #[derive(Debug, PartialEq, Eq)]
 enum SenderError {
-    /// No credential at all, so no sender can be attributed. Every delivered
+    /// No credential at all, so no sender can be attributed. The caller decides
-    /// message must carry an explicit sender.
+    /// what this means per conversation class: dropped for a group, delivered
+    /// without a sender for an anonymous PrivateV1 intro.
     Missing,
     /// Credential bytes were not valid hex.
     NotHex,
@@ -278,10 +280,11 @@ enum SenderError {
 /// Decode and verify a message's sender from its credential, checked against the
 /// account → device directory (our account store).
 ///
-/// `Ok(sender)` — deliver with the sender; its `account` is set only when the
+/// `Ok(sender)` — the sender was attributed; its `account` is set only when the
-/// directory confirmed the device, so it is always verified. `Err` — drop the
+/// directory confirmed the device, so it is always verified. `Err` — no sender
-/// message (including when no credential is present, since every delivered
+/// could be attributed (see [`SenderError`]). Whether an unattributed message is
-/// message must carry an explicit sender).
+/// dropped or delivered without a sender is decided by [`message_sender`]
+/// according to the conversation class.
 fn decode_sender(
     directory: &impl AccountDirectory,
     encoded: &[u8],
@@ -328,11 +331,36 @@ fn decode_sender(
     }
 }
 
+/// Resolve the sender to attach to a received message, honouring the
+/// conversation class. Returns `None` to drop the message, `Some(None)` to
+/// deliver it with no sender (an anonymous PrivateV1 message, which binds no
+/// credential by design), and `Some(Some(sender))` for a credential-bearing
+/// message whose sender verified.
+fn message_sender(
+    directory: &impl AccountDirectory,
+    encoded_credential: &[u8],
+    class: ConversationClass,
+) -> Option<Option<MessageSender>> {
+    match decode_sender(directory, encoded_credential) {
+        Ok(sender) => Some(Some(sender)),
+        // PrivateV1 is an out-of-band X3DH intro and attaches no credential, so
+        // surface its messages with no sender. For any other class an absent
+        // credential is a protocol violation, dropped along with every other
+        // unverifiable-sender case.
+        Err(SenderError::Missing) if class == ConversationClass::Private => Some(None),
+        Err(_) => None,
+    }
+}
+
 fn convo_events(outcome: ConvoOutcome, directory: &impl AccountDirectory) -> Vec<Event> {
-    let ConvoOutcome { convo_id, content } = outcome;
+    let ConvoOutcome {
+        convo_id,
+        content,
+        class,
+    } = outcome;
     content
         .and_then(|c| {
-            let sender = decode_sender(directory, &c.encoded_credential).ok()?;
+            let sender = message_sender(directory, &c.encoded_credential, class)?;
             Some(Event::MessageReceived {
                 convo_id: Arc::from(convo_id),
                 content: c.bytes,
@@ -355,7 +383,8 @@ fn inbox_events(outcome: InboxOutcome, directory: &impl AccountDirectory) -> Vec
         class: new_conversation.class,
     });
     if let Some(c) = initial.and_then(|co| co.content)
-        && let Ok(sender) = decode_sender(directory, &c.encoded_credential)
+        && let Some(sender) =
+            message_sender(directory, &c.encoded_credential, new_conversation.class)
     {
         events.push(Event::MessageReceived {
             convo_id: Arc::clone(&id),

crates/client/src/event.rs

@@ -32,11 +32,13 @@ pub enum Event {
         convo_id: Arc<str>,
         class: ConversationClass,
     },
-    /// User content arrived on an existing conversation.
+    /// User content arrived on an existing conversation. `sender` is `None` for
+    /// an anonymous PrivateV1 message: that conversation is an out-of-band X3DH
+    /// intro and binds no sender credential, so no identity can be resolved.
     MessageReceived {
         convo_id: Arc<str>,
         content: Vec<u8>,
-        sender: MessageSender,
+        sender: Option<MessageSender>,
     },
     InboundError {
         message: String,

crates/client/src/lib.rs

@@ -14,7 +14,7 @@ pub use event::{Event, MessageSender};
 
 // Re-export types callers need to interact with ChatClient.
 pub use libchat::{
-    AddressedEnvelope, ChatStore, ConversationClass, ConversationId, DeliveryService,
+    AddressedEnvelope, ChatStorage, ChatStore, ConversationClass, ConversationId, DeliveryService,
     IdentityProvider, RegistrationService, StorageConfig,
 };
 

crates/client/tests/saro_and_raya.rs

@@ -130,7 +130,9 @@ fn direct_v1_standalone_integration() {
         .expect("payload mismatch");
     expect_event(&raya_events, "MessageReceived", |e| match e {
         Event::MessageReceived {
-            content, sender, ..
+            content,
+            sender: Some(sender),
+            ..
         } => {
             assert_eq!(content.as_slice(), b"Hey from saro");
             // saro associated an account and published a matching bundle, so the
@@ -173,7 +175,7 @@ fn saro_raya_message_exchange() {
         Event::MessageReceived {
             convo_id,
             content,
-            sender,
+            sender: Some(sender),
         } => {
             assert_eq!(convo_id, raya_convo_id);
             assert_eq!(content.as_slice(), b"hello raya");
@@ -229,6 +231,56 @@ fn saro_raya_message_exchange() {
     assert_eq!(raya.list_conversations().unwrap().len(), 1);
 }
 
+/// PrivateV1 (intro-bundle) is an out-of-band X3DH intro that binds no sender
+/// credential, so its messages must still surface — with no sender — rather
+/// than be dropped. Covers both receive paths: the recipient's initial message
+/// (inbox) and the reply on the established conversation (convo).
+#[test]
+fn private_v1_integration() {
+    let bus = MessageBus::default();
+    let reg_service = EphemeralRegistry::new();
+
+    let (mut saro, saro_events) =
+        create_test_client(bus.clone(), reg_service.clone()).expect("client create");
+    let (mut raya, raya_events) =
+        create_test_client(bus.clone(), reg_service.clone()).expect("client create");
+
+    let raya_bundle = raya.create_intro_bundle().expect("intro bundle");
+    saro.create_conversation(&raya_bundle, b"hello raya")
+        .expect("convo create");
+
+    let raya_convo_id = expect_event(&raya_events, "ConversationStarted", |e| match e {
+        Event::ConversationStarted { convo_id, .. } => Ok(convo_id),
+        other => Err(other),
+    });
+    expect_event(&raya_events, "MessageReceived", |e| match e {
+        Event::MessageReceived {
+            content, sender, ..
+        } => {
+            assert_eq!(content.as_slice(), b"hello raya");
+            assert!(
+                sender.is_none(),
+                "PrivateV1 message must surface with no sender"
+            );
+            Ok(())
+        }
+        other => Err(other),
+    });
+
+    raya.send_message(&raya_convo_id, b"hi saro")
+        .expect("reply");
+    expect_event(&saro_events, "MessageReceived", |e| match e {
+        Event::MessageReceived {
+            content, sender, ..
+        } => {
+            assert_eq!(content.as_slice(), b"hi saro");
+            assert!(sender.is_none());
+            Ok(())
+        }
+        other => Err(other),
+    });
+}
+
 #[derive(Debug)]
 struct FailingDelivery {
     inbound_tx: Sender<Vec<u8>>,