diff --git a/src/app/mod.rs b/src/app/mod.rs index 322ea83..3bc8727 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -123,8 +123,8 @@ struct ChatEntry { /// for any future system-generated lines. from: Option, /// Optional file attachment descriptor. The bytes (if fetched) live in - /// `AppState.attachment_data` keyed by `attachment.id`; the entry only holds - /// the descriptor so history stays cheap. + /// `AppState.attachments` keyed by `(author, attachment.id)`; the entry only + /// holds the descriptor so history stays cheap. attachment: Option, } @@ -2272,9 +2272,22 @@ fn push_chat(messages: &mut Vec, entry: ChatEntry) { } } -/// Find the sender id + descriptor for a received attachment by its id, so a -/// fetch can be addressed. Returns `None` for our own attachments or an unknown -/// id. +/// Pick the default save-dialog filename for an attachment, matched by the FULL +/// `(author, id)` key — not the bare id — so a peer reusing another sender's id +/// can't supply the filename (and extension) for a different line (Tier C F-12 +/// metadata residual). Falls back to "download" if the line is gone. Matches own +/// and received lines alike (our own `from` = `self_id` parses to the key author). +fn attachment_default_name(messages: &[ChatEntry], key: AttachmentKey) -> String { + messages + .iter() + .find_map(|m| { + let att = m.attachment.as_ref()?; + let from = m.from.as_ref()?.parse::().ok()?; + (att.id == key.1 && from == key.0).then(|| att.name.clone()) + }) + .unwrap_or_else(|| "download".to_string()) +} + /// Find the chat-attachment descriptor for an exact `(author, id)` key among /// received (non-own) messages. Matching on the author too — not just the id — /// means a peer reusing another sender's id can't redirect the fetch to the @@ -2327,16 +2340,7 @@ fn save_attachment_task(state: &AppState, key: AttachmentKey) -> Task for Icon { #[cfg(test)] mod tests { use super::{ - format_duration, initial_window_position, reconnect_attempt_chime, reconnected_chime, - set_peer_gate_config, set_peer_volume_config, AppConfig, AppState, AttachmentCache, - AttachmentState, ChatEntry, GateMeter, METER_MAX, + attachment_default_name, format_duration, initial_window_position, reconnect_attempt_chime, + reconnected_chime, set_peer_gate_config, set_peer_volume_config, AppConfig, AppState, + AttachmentCache, AttachmentState, ChatEntry, GateMeter, METER_MAX, }; use iroh::SecretKey; @@ -5984,6 +5988,36 @@ mod tests { assert!(cache.get(&k).is_some()); } + #[test] + fn attachment_default_name_matches_full_key_not_bare_id() { + // Two chat lines carry the SAME attachment id but come from different + // senders. The save-dialog default filename must be the one belonging to + // the clicked (author, id) — not whichever line happens to match the bare + // id first (Tier C F-12 metadata residual). + let victim = SecretKey::generate().public(); + let attacker = SecretKey::generate().public(); + let shared_id = [7u8; 32]; + let mk = |from: iroh::EndpointId, fname: &str| ChatEntry { + name: "Peer".to_string(), + text: String::new(), + mine: false, + from: Some(from.to_string()), + attachment: Some(crate::files::ChatAttachment { + name: fname.to_string(), + size: 3, + kind: crate::files::AttachmentKind::File, + id: shared_id, + }), + }; + // Attacker's line is FIRST in history, so a bare-id scan would pick it. + let messages = vec![mk(attacker, "evil.sh"), mk(victim, "report.pdf")]; + assert_eq!(attachment_default_name(&messages, (victim, shared_id)), "report.pdf"); + assert_eq!(attachment_default_name(&messages, (attacker, shared_id)), "evil.sh"); + // Unknown line → safe fallback. + let unknown = SecretKey::generate().public(); + assert_eq!(attachment_default_name(&messages, (unknown, shared_id)), "download"); + } + #[test] fn reset_room_state_clears_all_room_scoped_state() { let mut state = AppState::default(); diff --git a/src/core/mod.rs b/src/core/mod.rs index 930c264..42b647d 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -805,7 +805,8 @@ fn should_auto_fetch(is_image: bool, author_in_roster: bool, already_inflight: b /// Fetch a chat attachment's bytes from `from` over the file plane in a detached /// task, then report the result to the UI via [`UiEvent::AttachmentReady`] / -/// [`UiEvent::AttachmentFailed`] keyed by the attachment id. For images +/// [`UiEvent::AttachmentFailed`], tagged with `from` so the UI keys the bytes by +/// `(author, id)` and can't alias a same-id attachment from another sender. For images /// (`is_image`) the bytes are defensively re-validated (decodable + within pixel /// limits) before being handed to the renderer; an "Image" that doesn't decode is /// reported as a failure rather than rendered. `guard` is `Some` for bounded