feat: make sender always exist in received message

This commit is contained in:
kaichaosun 2026-06-24 10:51:33 +08:00
parent 558fb986e8
commit 59612e540b
No known key found for this signature in database
GPG Key ID: 223E0F992F4F03BF
3 changed files with 35 additions and 25 deletions

View File

@ -260,6 +260,9 @@ fn account_key_from_hex(addr: &str) -> Option<Ed25519VerifyingKey> {
/// Why a message's sender could not be accepted, so the message is dropped. /// Why a message's sender could not be accepted, so the message is dropped.
#[derive(Debug, PartialEq, Eq)] #[derive(Debug, PartialEq, Eq)]
enum SenderError { enum SenderError {
/// No credential at all, so no sender can be attributed. Every delivered
/// message must carry an explicit sender.
Missing,
/// Credential bytes were not valid hex. /// Credential bytes were not valid hex.
NotHex, NotHex,
/// Credential bytes did not decode to a delegate credential. /// Credential bytes did not decode to a delegate credential.
@ -275,17 +278,17 @@ enum SenderError {
/// Decode and verify a message's sender from its credential, checked against the /// Decode and verify a message's sender from its credential, checked against the
/// account → device directory (our account store). /// account → device directory (our account store).
/// ///
/// `Ok(None)` — deliver, but the sender is unknown (no credential, e.g. a /// `Ok(sender)` — deliver with the sender; its `account` is set only when the
/// PrivateV1 1:1 message). `Ok(Some(sender))` — deliver with the sender; its /// directory confirmed the device, so it is always verified. `Err` — drop the
/// `account` is set only when the directory confirmed the device, so it is /// message (including when no credential is present, since every delivered
/// always verified. `Err` — drop the message. /// message must carry an explicit sender).
fn decode_sender( fn decode_sender(
directory: &impl AccountDirectory, directory: &impl AccountDirectory,
encoded: &[u8], encoded: &[u8],
) -> Result<Option<MessageSender>, SenderError> { ) -> Result<MessageSender, SenderError> {
// No credential (e.g. the PrivateV1 placeholder) asserts no account mapping. // No credential at all: there is no sender to attribute, so drop it.
if encoded.is_empty() { if encoded.is_empty() {
return Ok(None); return Err(SenderError::Missing);
} }
let Ok(data) = hex::decode(encoded) else { let Ok(data) = hex::decode(encoded) else {
tracing::warn!("sender credential is not valid hex; dropping message"); tracing::warn!("sender credential is not valid hex; dropping message");
@ -301,10 +304,10 @@ fn decode_sender(
let device = hex::encode(cred.delegate_id().as_ref()); let device = hex::encode(cred.delegate_id().as_ref());
// An unassociated delegate asserts no account → device mapping. // An unassociated delegate asserts no account → device mapping.
let Some(account_addr) = cred.account_addr() else { let Some(account_addr) = cred.account_addr() else {
return Ok(Some(MessageSender { return Ok(MessageSender {
account: None, account: None,
local_identity: IdentId::new(device), local_identity: IdentId::new(device),
})); });
}; };
let Some(account_key) = account_key_from_hex(account_addr) else { let Some(account_key) = account_key_from_hex(account_addr) else {
tracing::warn!( tracing::warn!(
@ -314,10 +317,10 @@ fn decode_sender(
return Err(SenderError::AccountNotAKey); return Err(SenderError::AccountNotAKey);
}; };
match directory.fetch(&account_key) { match directory.fetch(&account_key) {
Ok(Some(set)) if set.devices.iter().any(|d| d == &device) => Ok(Some(MessageSender { Ok(Some(set)) if set.devices.iter().any(|d| d == &device) => Ok(MessageSender {
account: Some(IdentId::new(account_addr.to_string())), account: Some(IdentId::new(account_addr.to_string())),
local_identity: IdentId::new(device), local_identity: IdentId::new(device),
})), }),
_ => { _ => {
tracing::warn!(account_addr, %device, "account → device mapping is wrong or unconfirmable; dropping message"); tracing::warn!(account_addr, %device, "account → device mapping is wrong or unconfirmable; dropping message");
Err(SenderError::Unverified) Err(SenderError::Unverified)
@ -441,10 +444,10 @@ mod sender_check_tests {
let cred = DelegateCredential::associated(&device, &hex::encode(account.as_ref())); let cred = DelegateCredential::associated(&device, &hex::encode(account.as_ref()));
assert_eq!( assert_eq!(
decode_sender(&dir, &encoded(cred)), decode_sender(&dir, &encoded(cred)),
Ok(Some(MessageSender { Ok(MessageSender {
account: Some(local_id(&account)), account: Some(local_id(&account)),
local_identity: local_id(&device), local_identity: local_id(&device),
})) })
); );
} }
@ -471,10 +474,10 @@ mod sender_check_tests {
let cred = DelegateCredential::unassociated(&device); let cred = DelegateCredential::unassociated(&device);
assert_eq!( assert_eq!(
decode_sender(&dir, &encoded(cred)), decode_sender(&dir, &encoded(cred)),
Ok(Some(MessageSender { Ok(MessageSender {
account: None, account: None,
local_identity: local_id(&device), local_identity: local_id(&device),
})) })
); );
} }
@ -509,12 +512,12 @@ mod sender_check_tests {
); );
} }
/// No credential at all (e.g. the PrivateV1 placeholder) asserts no account /// No credential at all (e.g. the PrivateV1 placeholder) leaves no sender to
/// mapping and is delivered with no sender. /// attribute, so the message is dropped.
#[test] #[test]
fn empty_credential_has_no_sender() { fn empty_credential_is_dropped() {
let dir = FakeDir::default(); let dir = FakeDir::default();
assert_eq!(decode_sender(&dir, b""), Ok(None)); assert_eq!(decode_sender(&dir, b""), Err(SenderError::Missing));
} }
/// Bytes that aren't a well-formed credential leave the sender's mapping /// Bytes that aren't a well-formed credential leave the sender's mapping

View File

@ -36,7 +36,7 @@ pub enum Event {
MessageReceived { MessageReceived {
convo_id: Arc<str>, convo_id: Arc<str>,
content: Vec<u8>, content: Vec<u8>,
sender: Option<MessageSender>, sender: MessageSender,
}, },
InboundError { InboundError {
message: String, message: String,

View File

@ -106,8 +106,14 @@ fn direct_v1_standalone_integration() {
raya_delegate.associate(hex::encode(raya_account.public_key().as_ref())); raya_delegate.associate(hex::encode(raya_account.public_key().as_ref()));
publish_device_bundle(&mut reg_service, &raya_account, raya_delegate.public_key()); publish_device_bundle(&mut reg_service, &raya_account, raya_delegate.public_key());
let (mut saro, _saro_events) = // Build saro's client with its associated delegate so its outbound messages
create_test_client(bus.clone(), reg_service.clone()).expect("client create"); // carry a credential the receiver can verify against the published bundle.
let (mut saro, _saro_events) = ChatClientBuilder::new()
.ident(saro_delegate)
.transport(InProcessDelivery::new(bus.clone()))
.registration(reg_service.clone())
.build()
.expect("client create");
let (raya, raya_events) = let (raya, raya_events) =
create_test_client(bus.clone(), reg_service.clone()).expect("client create"); create_test_client(bus.clone(), reg_service.clone()).expect("client create");
@ -129,7 +135,6 @@ fn direct_v1_standalone_integration() {
assert_eq!(content.as_slice(), b"Hey from saro"); assert_eq!(content.as_slice(), b"Hey from saro");
// saro associated an account and published a matching bundle, so the // saro associated an account and published a matching bundle, so the
// sender surfaces with a verified account and its device. // sender surfaces with a verified account and its device.
let sender = sender.expect("verified sender present");
assert_eq!( assert_eq!(
sender.account.as_ref().map(|a| a.as_str()), sender.account.as_ref().map(|a| a.as_str()),
Some(saro_account_id.as_str()) Some(saro_account_id.as_str())
@ -172,8 +177,10 @@ fn saro_raya_message_exchange() {
} => { } => {
assert_eq!(convo_id, raya_convo_id); assert_eq!(convo_id, raya_convo_id);
assert_eq!(content.as_slice(), b"hello raya"); assert_eq!(content.as_slice(), b"hello raya");
// PrivateV1 1:1 carries no credential, so there is no sender. // saro's delegate is unassociated, so the sender surfaces its device
assert!(sender.is_none()); // but claims no account.
assert!(sender.account.is_none());
assert!(!sender.local_identity.as_str().is_empty());
Ok(()) Ok(())
} }
other => Err(other), other => Err(other),