fix(security): address Codex F-02/F-12 audit — save-filename alias + doc nits

Codex audit of 381e00b (f02-f12-audit-2026-06-26) found no P1/P2 regression
and verified the F-12 key round-trip invariant sound (iroh EndpointId
Display/FromStr are exact inverses). Acting on the one real P3 + nits:

- P3: save_attachment_task picked the dialog's DEFAULT FILENAME by bare
  attachment id, so a peer reusing a victim's id could mislabel the save
  with another sender's name/extension (bytes were already author-keyed and
  correct — this was a metadata residual, not content aliasing). Extracted a
  pure `attachment_default_name` that matches the full (author, id) key, like
  find_attachment_source. +1 unit test (closes the audit's coverage gap).
- Doc nits: refreshed the stale `attachment_data` reference on ChatEntry,
  the "keyed by attachment id" note on spawn_attachment_fetch, and a
  duplicated doc block above find_attachment_source.

DEFERRED (user decision pending): the P3 judgement call — pending_plays /
invalid_audio / clip playing_id stay bare-id keyed, so duplicate-id audio
rows share play/seek/invalid state (cosmetic; bytes played are still
author-keyed and correct). Fully closing it means threading AttachmentKey
through the clip player.

424 lib tests, clippy --all-targets clean, release build green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/app/mod.rs

@@ -123,8 +123,8 @@ struct ChatEntry {
     /// for any future system-generated lines.
     from: Option<String>,
     /// 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<crate::files::ChatAttachment>,
 }
 
@@ -2272,9 +2272,22 @@ fn push_chat(messages: &mut Vec<ChatEntry>, 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::<EndpointId>().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<AppMessage
         return Task::none();
     };
     let data = data.clone();
-    let default_name = state
-        .chat_messages
-        .iter()
-        .find_map(|m| {
-            m.attachment
-                .as_ref()
-                .filter(|a| a.id == key.1)
-                .map(|a| a.name.clone())
-        })
-        .unwrap_or_else(|| "download".to_string());
+    let default_name = attachment_default_name(&state.chat_messages, key);
     Task::perform(
         async move {
             let handle = rfd::AsyncFileDialog::new()
@@ -5893,9 +5897,9 @@ impl Program<AppMessage> 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();

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