fix(security): bound + author-key chat attachment cache (Tier C F-02/F-12)
The chat-attachment result cache (`attachment_data` + `image_handle_cache`) was keyed by attachment id alone and only cleared on room-leave, so an authenticated insider could (F-02) stream distinct attachments to grow it without bound, and (F-12) reuse a victim's attachment id to alias displayed/ saved bytes — the id is attacker-chosen, so a signature only proves keypair ownership, not a distinct human. F-12: thread the author (`from: EndpointId`) back through the `AttachmentReady`/`AttachmentFailed` core→UI events (the fetch task already holds it) and key all attachment result state on `(author, id)`: - new `AttachmentKey = (EndpointId, AttachmentId)`; - `attachment_data` + `image_handle_cache` fold into one `AttachmentCache`; - `pending_saves` and the `SaveAttachment`/`PlayAudio` messages re-keyed, so the save/fetch dispatch can't be redirected to the wrong sender's line; - `find_attachment_source` now matches author AND id; - the render path resolves each line's key from `ChatEntry.from`. F-02: `AttachmentCache` is bounded (`ATTACHMENT_CACHE_CAP = 64`) with insertion-order eviction. True LRU is impossible because iced's `view` borrows `&self` and so can't reorder on a render read; the generous cap means a normal session never evicts and the newest (on-screen) entries are always retained — only an abusive stream hits the bound. Deliberately id-keyed (cosmetic only, documented): the clip player's `playing_id`, `pending_plays`, `invalid_audio` — they're coupled to the id-keyed clip player, and the bytes actually played come from the author-keyed cache, so content is always correct. No gossip/wire/protocol change (UiEvent is in-process), no new deps. +6 unit tests (cache eviction, replace-keeps-position, same-id/distinct-author non-aliasing, is_ready/handle/clear, cap-zero clamp). 423 lib tests, clippy --all-targets clean, release build green. TESTS-GREEN-ONLY. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+278
-99
@@ -23,7 +23,7 @@ use iced::{
|
|||||||
Point, Rectangle, Renderer, Size,
|
Point, Rectangle, Renderer, Size,
|
||||||
};
|
};
|
||||||
use iroh::EndpointId;
|
use iroh::EndpointId;
|
||||||
use std::collections::{HashMap, HashSet};
|
use std::collections::{HashMap, HashSet, VecDeque};
|
||||||
use std::sync::{Arc, OnceLock};
|
use std::sync::{Arc, OnceLock};
|
||||||
use tokio::sync::Mutex;
|
use tokio::sync::Mutex;
|
||||||
|
|
||||||
@@ -137,6 +137,91 @@ enum AttachmentState {
|
|||||||
Failed(String),
|
Failed(String),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Identifies one fetched attachment by BOTH the authoring peer and the
|
||||||
|
/// attachment id. The id is attacker-chosen, so a malicious peer can reuse a
|
||||||
|
/// victim's id to alias displayed/saved content; keying on the author too means
|
||||||
|
/// each chat line resolves only its own sender's bytes (Tier C F-12).
|
||||||
|
type AttachmentKey = (EndpointId, crate::files::AttachmentId);
|
||||||
|
|
||||||
|
/// Cap on retained attachment results so an insider streaming distinct
|
||||||
|
/// attachments can't grow the cache without bound (Tier C F-02). Sized well
|
||||||
|
/// above any realistic on-screen image working set.
|
||||||
|
const ATTACHMENT_CACHE_CAP: usize = 64;
|
||||||
|
|
||||||
|
/// One cached attachment result: its fetch state plus, for ready images, the
|
||||||
|
/// pre-built iced image handle (built once on arrival, not per redraw — the
|
||||||
|
/// e917c53 flicker fix). `handle` is `None` for files and failures.
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
struct AttachmentEntry {
|
||||||
|
state: AttachmentState,
|
||||||
|
handle: Option<iced::widget::image::Handle>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Bounded store of fetched chat-attachment results, keyed by [`AttachmentKey`].
|
||||||
|
///
|
||||||
|
/// Eviction is insertion-order (oldest first), NOT true LRU: iced's `view`
|
||||||
|
/// borrows `&self`, so the render read path cannot reorder an access-ordered
|
||||||
|
/// cache. With a generous cap the newest entries — the ones actually on screen —
|
||||||
|
/// are always retained, so a normal session never evicts; only an abusive stream
|
||||||
|
/// of distinct attachments hits the bound (Tier C F-02).
|
||||||
|
///
|
||||||
|
/// Construct via [`AttachmentCache::new`] — there is deliberately no `Default`,
|
||||||
|
/// because a zero cap would make `insert` evict endlessly.
|
||||||
|
#[derive(Debug)]
|
||||||
|
struct AttachmentCache {
|
||||||
|
entries: HashMap<AttachmentKey, AttachmentEntry>,
|
||||||
|
/// Keys in insertion order; the front is the eviction candidate. Holds
|
||||||
|
/// exactly the present keys (one entry each), so it is bounded by `cap`.
|
||||||
|
order: VecDeque<AttachmentKey>,
|
||||||
|
cap: usize,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl AttachmentCache {
|
||||||
|
fn new(cap: usize) -> Self {
|
||||||
|
Self { entries: HashMap::new(), order: VecDeque::new(), cap: cap.max(1) }
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Insert or replace an entry. A brand-new key evicts the oldest entries
|
||||||
|
/// until there is room; replacing an existing key keeps its position (and so
|
||||||
|
/// its age), only updating the value.
|
||||||
|
fn insert(&mut self, key: AttachmentKey, state: AttachmentState, handle: Option<iced::widget::image::Handle>) {
|
||||||
|
if !self.entries.contains_key(&key) {
|
||||||
|
while self.entries.len() >= self.cap {
|
||||||
|
match self.order.pop_front() {
|
||||||
|
Some(old) => {
|
||||||
|
self.entries.remove(&old);
|
||||||
|
}
|
||||||
|
None => break,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
self.order.push_back(key);
|
||||||
|
}
|
||||||
|
self.entries.insert(key, AttachmentEntry { state, handle });
|
||||||
|
}
|
||||||
|
|
||||||
|
fn get(&self, key: &AttachmentKey) -> Option<&AttachmentState> {
|
||||||
|
self.entries.get(key).map(|e| &e.state)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn handle(&self, key: &AttachmentKey) -> Option<&iced::widget::image::Handle> {
|
||||||
|
self.entries.get(key).and_then(|e| e.handle.as_ref())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_ready(&self, key: &AttachmentKey) -> bool {
|
||||||
|
matches!(self.entries.get(key), Some(AttachmentEntry { state: AttachmentState::Ready(_), .. }))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn clear(&mut self) {
|
||||||
|
self.entries.clear();
|
||||||
|
self.order.clear();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
fn len(&self) -> usize {
|
||||||
|
self.entries.len()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Cap on retained chat history so a long call can't grow it without bound.
|
/// Cap on retained chat history so a long call can't grow it without bound.
|
||||||
const CHAT_HISTORY_MAX: usize = 300;
|
const CHAT_HISTORY_MAX: usize = 300;
|
||||||
|
|
||||||
@@ -289,12 +374,15 @@ pub enum AppMessage {
|
|||||||
PickAttachmentFile,
|
PickAttachmentFile,
|
||||||
/// Result of the attach picker: (filename, bytes), or None if cancelled.
|
/// Result of the attach picker: (filename, bytes), or None if cancelled.
|
||||||
AttachmentFilePicked(Option<(String, Vec<u8>)>),
|
AttachmentFilePicked(Option<(String, Vec<u8>)>),
|
||||||
/// Save (downloading first if needed) a received attachment to disk.
|
/// Save (downloading first if needed) a received attachment to disk. Carries
|
||||||
SaveAttachment(crate::files::AttachmentId),
|
/// the full `(author, id)` key so the correct sender's bytes are fetched and
|
||||||
|
/// saved even if another peer reused the same attachment id (Tier C F-12).
|
||||||
|
SaveAttachment(AttachmentKey),
|
||||||
/// Result of the async save dialog: a status line to show, or None if cancelled.
|
/// Result of the async save dialog: a status line to show, or None if cancelled.
|
||||||
AttachmentSaved(Option<String>),
|
AttachmentSaved(Option<String>),
|
||||||
/// Fetch (if needed) and start an inline audio attachment.
|
/// Fetch (if needed) and start an inline audio attachment. Carries the full
|
||||||
PlayAudio(crate::files::AttachmentId),
|
/// `(author, id)` key (see [`AppMessage::SaveAttachment`]).
|
||||||
|
PlayAudio(AttachmentKey),
|
||||||
PauseAudio,
|
PauseAudio,
|
||||||
ResumeAudio,
|
ResumeAudio,
|
||||||
SeekAudio(crate::files::AttachmentId, f32),
|
SeekAudio(crate::files::AttachmentId, f32),
|
||||||
@@ -451,15 +539,14 @@ pub struct AppState {
|
|||||||
/// Room text-chat history (newest last) and the pending input line.
|
/// Room text-chat history (newest last) and the pending input line.
|
||||||
chat_messages: Vec<ChatEntry>,
|
chat_messages: Vec<ChatEntry>,
|
||||||
chat_input: String,
|
chat_input: String,
|
||||||
/// Fetched/failed state for chat attachments, keyed by attachment id.
|
/// Fetched/failed bytes + cached image handles for chat attachments, keyed by
|
||||||
/// Session-only (cleared on leave); never persisted.
|
/// `(author, id)` and bounded. Session-only (cleared on leave); never
|
||||||
attachment_data: HashMap<crate::files::AttachmentId, AttachmentState>,
|
/// persisted. (Tier C F-02 bound + F-12 author keying.)
|
||||||
/// Cached iced image handles for ready image attachments, keyed by id, so we
|
attachments: AttachmentCache,
|
||||||
/// don't re-upload to the GPU every redraw (the e917c53 avatar flicker fix).
|
/// Attachments the user asked to save before the bytes arrived; when the
|
||||||
image_handle_cache: HashMap<crate::files::AttachmentId, iced::widget::image::Handle>,
|
/// fetch completes a save dialog is opened for them. Keyed by `(author, id)`
|
||||||
/// Attachment ids the user asked to save before the bytes arrived; when the
|
/// so a same-id attachment from a different sender can't trigger the save.
|
||||||
/// fetch completes a save dialog is opened for them.
|
pending_saves: HashSet<AttachmentKey>,
|
||||||
pending_saves: std::collections::HashSet<crate::files::AttachmentId>,
|
|
||||||
/// Clip ids waiting for the existing attachment fetch path to return bytes.
|
/// Clip ids waiting for the existing attachment fetch path to return bytes.
|
||||||
pending_plays: HashSet<crate::files::AttachmentId>,
|
pending_plays: HashSet<crate::files::AttachmentId>,
|
||||||
/// Filename-hinted audio whose fetched bytes or decoder validation failed;
|
/// Filename-hinted audio whose fetched bytes or decoder validation failed;
|
||||||
@@ -536,8 +623,7 @@ impl AppState {
|
|||||||
self.locally_muted.clear();
|
self.locally_muted.clear();
|
||||||
self.chat_messages.clear();
|
self.chat_messages.clear();
|
||||||
self.chat_input.clear();
|
self.chat_input.clear();
|
||||||
self.attachment_data.clear();
|
self.attachments.clear();
|
||||||
self.image_handle_cache.clear();
|
|
||||||
self.pending_saves.clear();
|
self.pending_saves.clear();
|
||||||
self.pending_plays.clear();
|
self.pending_plays.clear();
|
||||||
self.invalid_audio.clear();
|
self.invalid_audio.clear();
|
||||||
@@ -658,9 +744,8 @@ impl Default for AppState {
|
|||||||
recording: false,
|
recording: false,
|
||||||
recording_started: None,
|
recording_started: None,
|
||||||
chat_messages: Vec::new(),
|
chat_messages: Vec::new(),
|
||||||
attachment_data: HashMap::new(),
|
attachments: AttachmentCache::new(ATTACHMENT_CACHE_CAP),
|
||||||
image_handle_cache: HashMap::new(),
|
pending_saves: HashSet::new(),
|
||||||
pending_saves: std::collections::HashSet::new(),
|
|
||||||
pending_plays: HashSet::new(),
|
pending_plays: HashSet::new(),
|
||||||
invalid_audio: HashSet::new(),
|
invalid_audio: HashSet::new(),
|
||||||
clip_player,
|
clip_player,
|
||||||
@@ -1186,31 +1271,31 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
UiEvent::AttachmentReady { id, data } => {
|
UiEvent::AttachmentReady { from, id, data } => {
|
||||||
// Bytes arrived. For images we can cache the iced handle now
|
// Bytes arrived for this specific (author, id). For images we
|
||||||
// (built once, not per redraw). If the user was waiting to save
|
// can cache the iced handle now (built once, not per redraw).
|
||||||
// this file, the save dialog is opened from update() below by
|
// If the user was waiting to save this file, the save dialog
|
||||||
// checking pending_saves — done lazily so this arm stays simple.
|
// is opened from update() below by checking pending_saves —
|
||||||
if crate::files::validate_image_bytes(&data).is_some() {
|
// done lazily so this arm stays simple.
|
||||||
state.image_handle_cache.insert(
|
let key = (from, id);
|
||||||
id,
|
let handle = crate::files::validate_image_bytes(&data)
|
||||||
iced::widget::image::Handle::from_bytes(data.clone()),
|
.is_some()
|
||||||
);
|
.then(|| iced::widget::image::Handle::from_bytes(data.clone()));
|
||||||
}
|
let needs_save = state.pending_saves.remove(&key);
|
||||||
let needs_save = state.pending_saves.remove(&id);
|
|
||||||
let needs_play = state.pending_plays.remove(&id);
|
let needs_play = state.pending_plays.remove(&id);
|
||||||
state.attachment_data.insert(id, AttachmentState::Ready(data));
|
state.attachments.insert(key, AttachmentState::Ready(data), handle);
|
||||||
if needs_play {
|
if needs_play {
|
||||||
play_ready_audio(state, id);
|
play_ready_audio(state, key);
|
||||||
}
|
}
|
||||||
if needs_save {
|
if needs_save {
|
||||||
return save_attachment_task(state, id);
|
return save_attachment_task(state, key);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
UiEvent::AttachmentFailed { id, error } => {
|
UiEvent::AttachmentFailed { from, id, error } => {
|
||||||
state.pending_saves.remove(&id);
|
let key = (from, id);
|
||||||
|
state.pending_saves.remove(&key);
|
||||||
state.pending_plays.remove(&id);
|
state.pending_plays.remove(&id);
|
||||||
state.attachment_data.insert(id, AttachmentState::Failed(error.clone()));
|
state.attachments.insert(key, AttachmentState::Failed(error.clone()), None);
|
||||||
state.status_message = format!("Attachment failed: {error}");
|
state.status_message = format!("Attachment failed: {error}");
|
||||||
}
|
}
|
||||||
UiEvent::ScreenShareStarted => {
|
UiEvent::ScreenShareStarted => {
|
||||||
@@ -1870,17 +1955,18 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
|||||||
id,
|
id,
|
||||||
};
|
};
|
||||||
// Keep our own bytes locally so we see our own attachment inline
|
// Keep our own bytes locally so we see our own attachment inline
|
||||||
// immediately (others fetch it off the file plane).
|
// immediately (others fetch it off the file plane). Cache under
|
||||||
if kind == crate::files::AttachmentKind::Image
|
// our own (author, id) key so the render path — which keys the
|
||||||
&& crate::files::validate_image_bytes(&bytes).is_some()
|
// line by `from` = self_id — finds them.
|
||||||
{
|
if let Ok(self_eid) = state.self_id.parse::<EndpointId>() {
|
||||||
|
let key = (self_eid, id);
|
||||||
|
let handle = (kind == crate::files::AttachmentKind::Image
|
||||||
|
&& crate::files::validate_image_bytes(&bytes).is_some())
|
||||||
|
.then(|| iced::widget::image::Handle::from_bytes(bytes.clone()));
|
||||||
state
|
state
|
||||||
.image_handle_cache
|
.attachments
|
||||||
.insert(id, iced::widget::image::Handle::from_bytes(bytes.clone()));
|
.insert(key, AttachmentState::Ready(bytes.clone()), handle);
|
||||||
}
|
}
|
||||||
state
|
|
||||||
.attachment_data
|
|
||||||
.insert(id, AttachmentState::Ready(bytes.clone()));
|
|
||||||
push_chat(
|
push_chat(
|
||||||
&mut state.chat_messages,
|
&mut state.chat_messages,
|
||||||
ChatEntry {
|
ChatEntry {
|
||||||
@@ -1898,21 +1984,18 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
AppMessage::SaveAttachment(id) => {
|
AppMessage::SaveAttachment(key) => {
|
||||||
// If we already have the bytes, save now; otherwise fetch from the
|
// If we already have the bytes, save now; otherwise fetch from the
|
||||||
// sender and save when AttachmentReady arrives (pending_saves).
|
// sender and save when AttachmentReady arrives (pending_saves). The
|
||||||
if matches!(state.attachment_data.get(&id), Some(AttachmentState::Ready(_))) {
|
// key's author half is the exact sender of the clicked line.
|
||||||
return save_attachment_task(state, id);
|
if state.attachments.is_ready(&key) {
|
||||||
} else if let Some((from, att)) = find_attachment_source(state, id) {
|
return save_attachment_task(state, key);
|
||||||
if let Ok(eid) = from.parse::<EndpointId>() {
|
} else if let Some(att) = find_attachment_source(state, key) {
|
||||||
state.pending_saves.insert(id);
|
state.pending_saves.insert(key);
|
||||||
state.status_message = format!("Downloading {}…", att.name);
|
state.status_message = format!("Downloading {}…", att.name);
|
||||||
let _ = state
|
let _ = state
|
||||||
.controller
|
.controller
|
||||||
.send(CoreCommand::FetchAttachment { from: eid, attachment: att });
|
.send(CoreCommand::FetchAttachment { from: key.0, attachment: att });
|
||||||
} else {
|
|
||||||
state.status_message = "Can't download: unknown sender.".to_string();
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
AppMessage::AttachmentSaved(msg) => {
|
AppMessage::AttachmentSaved(msg) => {
|
||||||
@@ -1920,21 +2003,18 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
|||||||
state.status_message = m;
|
state.status_message = m;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
AppMessage::PlayAudio(id) => {
|
AppMessage::PlayAudio(key) => {
|
||||||
if matches!(state.attachment_data.get(&id), Some(AttachmentState::Ready(_))) {
|
if state.attachments.is_ready(&key) {
|
||||||
play_ready_audio(state, id);
|
play_ready_audio(state, key);
|
||||||
} else if let Some((from, att)) = find_attachment_source(state, id) {
|
} else if let Some(att) = find_attachment_source(state, key) {
|
||||||
if let Ok(eid) = from.parse::<EndpointId>() {
|
// Repeated clicks while the transfer is pending must not launch
|
||||||
// Repeated clicks while the transfer is pending must not
|
// duplicate fetches. (pending_plays is id-keyed — it's coupled to
|
||||||
// launch duplicate fetches.
|
// the id-keyed clip player; same-id collisions are cosmetic.)
|
||||||
if state.pending_plays.insert(id) {
|
if state.pending_plays.insert(key.1) {
|
||||||
state.status_message = format!("Loading {}…", att.name);
|
state.status_message = format!("Loading {}…", att.name);
|
||||||
let _ = state
|
let _ = state
|
||||||
.controller
|
.controller
|
||||||
.send(CoreCommand::FetchAttachment { from: eid, attachment: att });
|
.send(CoreCommand::FetchAttachment { from: key.0, attachment: att });
|
||||||
}
|
|
||||||
} else {
|
|
||||||
state.status_message = "Can't play: unknown sender.".to_string();
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -2195,14 +2275,19 @@ fn push_chat(messages: &mut Vec<ChatEntry>, entry: ChatEntry) {
|
|||||||
/// Find the sender id + descriptor for a received attachment by its id, so a
|
/// 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
|
/// fetch can be addressed. Returns `None` for our own attachments or an unknown
|
||||||
/// id.
|
/// id.
|
||||||
|
/// 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
|
||||||
|
/// wrong line (Tier C F-12).
|
||||||
fn find_attachment_source(
|
fn find_attachment_source(
|
||||||
state: &AppState,
|
state: &AppState,
|
||||||
id: crate::files::AttachmentId,
|
key: AttachmentKey,
|
||||||
) -> Option<(String, crate::files::ChatAttachment)> {
|
) -> Option<crate::files::ChatAttachment> {
|
||||||
state.chat_messages.iter().find_map(|m| {
|
state.chat_messages.iter().find_map(|m| {
|
||||||
let att = m.attachment.as_ref()?;
|
let att = m.attachment.as_ref()?;
|
||||||
if att.id == id && !m.mine {
|
let from = m.from.as_ref()?;
|
||||||
Some((m.from.clone()?, att.clone()))
|
if att.id == key.1 && !m.mine && from.parse::<EndpointId>().ok() == Some(key.0) {
|
||||||
|
Some(att.clone())
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
@@ -2211,13 +2296,15 @@ fn find_attachment_source(
|
|||||||
|
|
||||||
/// Validate cached bytes and hand them to the independent clip player. A false
|
/// Validate cached bytes and hand them to the independent clip player. A false
|
||||||
/// filename hint falls back to the generic file chip without reaching rodio.
|
/// filename hint falls back to the generic file chip without reaching rodio.
|
||||||
fn play_ready_audio(state: &mut AppState, id: crate::files::AttachmentId) {
|
fn play_ready_audio(state: &mut AppState, key: AttachmentKey) {
|
||||||
let Some(AttachmentState::Ready(data)) = state.attachment_data.get(&id) else {
|
let Some(AttachmentState::Ready(data)) = state.attachments.get(&key) else {
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
let id = key.1;
|
||||||
if crate::files::is_probably_audio(data) {
|
if crate::files::is_probably_audio(data) {
|
||||||
|
let bytes = data.clone();
|
||||||
state.invalid_audio.remove(&id);
|
state.invalid_audio.remove(&id);
|
||||||
state.clip_player.play(id, data.clone());
|
state.clip_player.play(id, bytes);
|
||||||
} else {
|
} else {
|
||||||
state.invalid_audio.insert(id);
|
state.invalid_audio.insert(id);
|
||||||
state.status_message = "This attachment is not valid supported audio.".to_string();
|
state.status_message = "This attachment is not valid supported audio.".to_string();
|
||||||
@@ -2235,8 +2322,8 @@ fn play_ready_audio(state: &mut AppState, id: crate::files::AttachmentId) {
|
|||||||
/// the Linux `xdg-desktop-portal`/GTK backend wedges the dialog — Save/Cancel
|
/// the Linux `xdg-desktop-portal`/GTK backend wedges the dialog — Save/Cancel
|
||||||
/// stop responding. The file *picker* paths already use the async variant; this
|
/// stop responding. The file *picker* paths already use the async variant; this
|
||||||
/// is the one save path that must match.
|
/// is the one save path that must match.
|
||||||
fn save_attachment_task(state: &AppState, id: crate::files::AttachmentId) -> Task<AppMessage> {
|
fn save_attachment_task(state: &AppState, key: AttachmentKey) -> Task<AppMessage> {
|
||||||
let Some(AttachmentState::Ready(data)) = state.attachment_data.get(&id) else {
|
let Some(AttachmentState::Ready(data)) = state.attachments.get(&key) else {
|
||||||
return Task::none();
|
return Task::none();
|
||||||
};
|
};
|
||||||
let data = data.clone();
|
let data = data.clone();
|
||||||
@@ -2246,7 +2333,7 @@ fn save_attachment_task(state: &AppState, id: crate::files::AttachmentId) -> Tas
|
|||||||
.find_map(|m| {
|
.find_map(|m| {
|
||||||
m.attachment
|
m.attachment
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.filter(|a| a.id == id)
|
.filter(|a| a.id == key.1)
|
||||||
.map(|a| a.name.clone())
|
.map(|a| a.name.clone())
|
||||||
})
|
})
|
||||||
.unwrap_or_else(|| "download".to_string());
|
.unwrap_or_else(|| "download".to_string());
|
||||||
@@ -4382,7 +4469,14 @@ fn view(state: &AppState) -> Element<'_, AppMessage> {
|
|||||||
);
|
);
|
||||||
// Attachment row (indented under the message), if any.
|
// Attachment row (indented under the message), if any.
|
||||||
if let Some(att) = &m.attachment {
|
if let Some(att) = &m.attachment {
|
||||||
let data = state.attachment_data.get(&att.id);
|
// This line's cache key is (its author, the attachment id).
|
||||||
|
// `None` only for a system line or an unparseable author.
|
||||||
|
let key: Option<AttachmentKey> = m
|
||||||
|
.from
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|f| f.parse::<EndpointId>().ok())
|
||||||
|
.map(|eid| (eid, att.id));
|
||||||
|
let data = key.as_ref().and_then(|k| state.attachments.get(k));
|
||||||
let elem: Element<'_, AppMessage> =
|
let elem: Element<'_, AppMessage> =
|
||||||
if let Some(AttachmentState::Failed(e)) = data {
|
if let Some(AttachmentState::Failed(e)) = data {
|
||||||
text(format!("⚠ {} — {e}", att.name))
|
text(format!("⚠ {} — {e}", att.name))
|
||||||
@@ -4390,7 +4484,7 @@ fn view(state: &AppState) -> Element<'_, AppMessage> {
|
|||||||
.color(color_red)
|
.color(color_red)
|
||||||
.into()
|
.into()
|
||||||
} else if att.kind == crate::files::AttachmentKind::Image {
|
} else if att.kind == crate::files::AttachmentKind::Image {
|
||||||
match state.image_handle_cache.get(&att.id) {
|
match key.as_ref().and_then(|k| state.attachments.handle(k)) {
|
||||||
Some(handle) => iced::widget::image(handle.clone())
|
Some(handle) => iced::widget::image(handle.clone())
|
||||||
.width(iced::Length::Fixed(260.0))
|
.width(iced::Length::Fixed(260.0))
|
||||||
.into(),
|
.into(),
|
||||||
@@ -4419,7 +4513,7 @@ fn view(state: &AppState) -> Element<'_, AppMessage> {
|
|||||||
button(text("Pause").size(12)).on_press(AppMessage::PauseAudio)
|
button(text("Pause").size(12)).on_press(AppMessage::PauseAudio)
|
||||||
} else {
|
} else {
|
||||||
button(text("Play").size(12))
|
button(text("Play").size(12))
|
||||||
.on_press(AppMessage::PlayAudio(att.id))
|
.on_press_maybe(key.map(AppMessage::PlayAudio))
|
||||||
}
|
}
|
||||||
.style(b_style(
|
.style(b_style(
|
||||||
color_blue,
|
color_blue,
|
||||||
@@ -4447,7 +4541,7 @@ fn view(state: &AppState) -> Element<'_, AppMessage> {
|
|||||||
"Download"
|
"Download"
|
||||||
})
|
})
|
||||||
.size(12))
|
.size(12))
|
||||||
.on_press(AppMessage::SaveAttachment(att.id))
|
.on_press_maybe(key.map(AppMessage::SaveAttachment))
|
||||||
.style(b_style(
|
.style(b_style(
|
||||||
color_surface,
|
color_surface,
|
||||||
color_overlay,
|
color_overlay,
|
||||||
@@ -4493,7 +4587,7 @@ fn view(state: &AppState) -> Element<'_, AppMessage> {
|
|||||||
.size(12)
|
.size(12)
|
||||||
.color(color_text),
|
.color(color_text),
|
||||||
button(text(btn_label).size(12))
|
button(text(btn_label).size(12))
|
||||||
.on_press(AppMessage::SaveAttachment(att.id))
|
.on_press_maybe(key.map(AppMessage::SaveAttachment))
|
||||||
.style(b_style(
|
.style(b_style(
|
||||||
color_blue,
|
color_blue,
|
||||||
color_lavender,
|
color_lavender,
|
||||||
@@ -5800,11 +5894,96 @@ impl Program<AppMessage> for Icon {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::{
|
use super::{
|
||||||
format_duration, initial_window_position, reconnect_attempt_chime, reconnected_chime,
|
format_duration, initial_window_position, reconnect_attempt_chime, reconnected_chime,
|
||||||
set_peer_gate_config, set_peer_volume_config, AppConfig, AppState, AttachmentState,
|
set_peer_gate_config, set_peer_volume_config, AppConfig, AppState, AttachmentCache,
|
||||||
ChatEntry, GateMeter, METER_MAX,
|
AttachmentState, ChatEntry, GateMeter, METER_MAX,
|
||||||
};
|
};
|
||||||
use iroh::SecretKey;
|
use iroh::SecretKey;
|
||||||
|
|
||||||
|
/// Two distinct (peer, id) keys for cache tests; `id` may be shared to model
|
||||||
|
/// a malicious peer reusing a victim's attachment id (Tier C F-12).
|
||||||
|
fn peer_key(id: [u8; 32]) -> super::AttachmentKey {
|
||||||
|
(SecretKey::generate().public(), id)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn attachment_cache_evicts_oldest_when_full() {
|
||||||
|
let mut cache = AttachmentCache::new(2);
|
||||||
|
let k1 = peer_key([1u8; 32]);
|
||||||
|
let k2 = peer_key([2u8; 32]);
|
||||||
|
let k3 = peer_key([3u8; 32]);
|
||||||
|
cache.insert(k1, AttachmentState::Ready(vec![1]), None);
|
||||||
|
cache.insert(k2, AttachmentState::Ready(vec![2]), None);
|
||||||
|
assert_eq!(cache.len(), 2);
|
||||||
|
// Inserting a third NEW key evicts the oldest (k1), not the newest.
|
||||||
|
cache.insert(k3, AttachmentState::Ready(vec![3]), None);
|
||||||
|
assert_eq!(cache.len(), 2);
|
||||||
|
assert!(cache.get(&k1).is_none(), "oldest should be evicted");
|
||||||
|
assert!(cache.get(&k2).is_some());
|
||||||
|
assert!(cache.get(&k3).is_some());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn attachment_cache_replace_keeps_position_and_count() {
|
||||||
|
let mut cache = AttachmentCache::new(2);
|
||||||
|
let k1 = peer_key([1u8; 32]);
|
||||||
|
let k2 = peer_key([2u8; 32]);
|
||||||
|
cache.insert(k1, AttachmentState::Failed("pending".into()), None);
|
||||||
|
cache.insert(k2, AttachmentState::Ready(vec![2]), None);
|
||||||
|
// Replacing k1 (Failed -> Ready) must NOT bump it to newest; it stays the
|
||||||
|
// eviction candidate, and the count is unchanged (no order leak).
|
||||||
|
cache.insert(k1, AttachmentState::Ready(vec![1]), None);
|
||||||
|
assert_eq!(cache.len(), 2);
|
||||||
|
let k3 = peer_key([3u8; 32]);
|
||||||
|
cache.insert(k3, AttachmentState::Ready(vec![3]), None);
|
||||||
|
assert!(cache.get(&k1).is_none(), "replaced entry kept its old age");
|
||||||
|
assert!(cache.get(&k2).is_some());
|
||||||
|
assert!(cache.get(&k3).is_some());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn attachment_cache_same_id_distinct_authors_do_not_alias() {
|
||||||
|
// The F-12 core property: two peers sending the SAME attachment id keep
|
||||||
|
// separate bytes — one never overwrites or aliases the other.
|
||||||
|
let mut cache = AttachmentCache::new(8);
|
||||||
|
let shared_id = [7u8; 32];
|
||||||
|
let victim = peer_key(shared_id);
|
||||||
|
let attacker = peer_key(shared_id);
|
||||||
|
cache.insert(victim, AttachmentState::Ready(vec![1, 1, 1]), None);
|
||||||
|
cache.insert(attacker, AttachmentState::Ready(vec![9, 9, 9]), None);
|
||||||
|
assert_eq!(cache.len(), 2);
|
||||||
|
assert!(matches!(cache.get(&victim), Some(AttachmentState::Ready(b)) if b == &[1, 1, 1]));
|
||||||
|
assert!(matches!(cache.get(&attacker), Some(AttachmentState::Ready(b)) if b == &[9, 9, 9]));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn attachment_cache_is_ready_and_handle_and_clear() {
|
||||||
|
let mut cache = AttachmentCache::new(4);
|
||||||
|
let k = peer_key([5u8; 32]);
|
||||||
|
cache.insert(
|
||||||
|
k,
|
||||||
|
AttachmentState::Ready(vec![1]),
|
||||||
|
Some(iced::widget::image::Handle::from_bytes(vec![1])),
|
||||||
|
);
|
||||||
|
assert!(cache.is_ready(&k));
|
||||||
|
assert!(cache.handle(&k).is_some());
|
||||||
|
let failed = peer_key([6u8; 32]);
|
||||||
|
cache.insert(failed, AttachmentState::Failed("nope".into()), None);
|
||||||
|
assert!(!cache.is_ready(&failed));
|
||||||
|
assert!(cache.handle(&failed).is_none());
|
||||||
|
cache.clear();
|
||||||
|
assert_eq!(cache.len(), 0);
|
||||||
|
assert!(cache.get(&k).is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn attachment_cache_cap_zero_is_clamped_to_one() {
|
||||||
|
let mut cache = AttachmentCache::new(0);
|
||||||
|
let k = peer_key([1u8; 32]);
|
||||||
|
cache.insert(k, AttachmentState::Ready(vec![1]), None);
|
||||||
|
assert_eq!(cache.len(), 1);
|
||||||
|
assert!(cache.get(&k).is_some());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn reset_room_state_clears_all_room_scoped_state() {
|
fn reset_room_state_clears_all_room_scoped_state() {
|
||||||
let mut state = AppState::default();
|
let mut state = AppState::default();
|
||||||
@@ -5830,12 +6009,13 @@ mod tests {
|
|||||||
attachment: None,
|
attachment: None,
|
||||||
});
|
});
|
||||||
state.chat_input = "draft".to_string();
|
state.chat_input = "draft".to_string();
|
||||||
state.attachment_data.insert(attachment_id, AttachmentState::Ready(vec![1]));
|
let att_key = (peer, attachment_id);
|
||||||
state.image_handle_cache.insert(
|
state.attachments.insert(
|
||||||
attachment_id,
|
att_key,
|
||||||
iced::widget::image::Handle::from_bytes(vec![1]),
|
AttachmentState::Ready(vec![1]),
|
||||||
|
Some(iced::widget::image::Handle::from_bytes(vec![1])),
|
||||||
);
|
);
|
||||||
state.pending_saves.insert(attachment_id);
|
state.pending_saves.insert(att_key);
|
||||||
state.pending_plays.insert(attachment_id);
|
state.pending_plays.insert(attachment_id);
|
||||||
state.invalid_audio.insert(attachment_id);
|
state.invalid_audio.insert(attachment_id);
|
||||||
state.connecting.insert(peer);
|
state.connecting.insert(peer);
|
||||||
@@ -5854,8 +6034,7 @@ mod tests {
|
|||||||
assert!(state.locally_muted.is_empty());
|
assert!(state.locally_muted.is_empty());
|
||||||
assert!(state.chat_messages.is_empty());
|
assert!(state.chat_messages.is_empty());
|
||||||
assert!(state.chat_input.is_empty());
|
assert!(state.chat_input.is_empty());
|
||||||
assert!(state.attachment_data.is_empty());
|
assert!(state.attachments.len() == 0);
|
||||||
assert!(state.image_handle_cache.is_empty());
|
|
||||||
assert!(state.pending_saves.is_empty());
|
assert!(state.pending_saves.is_empty());
|
||||||
assert!(state.pending_plays.is_empty());
|
assert!(state.pending_plays.is_empty());
|
||||||
assert!(state.invalid_audio.is_empty());
|
assert!(state.invalid_audio.is_empty());
|
||||||
|
|||||||
@@ -133,11 +133,13 @@ pub enum UiEvent {
|
|||||||
/// string, used to key their avatar (W4).
|
/// string, used to key their avatar (W4).
|
||||||
ChatMessage { from: String, name: String, text: String, attachment: Option<crate::files::ChatAttachment> },
|
ChatMessage { from: String, name: String, text: String, attachment: Option<crate::files::ChatAttachment> },
|
||||||
/// An attachment's bytes are now available (auto-fetched for images, or
|
/// An attachment's bytes are now available (auto-fetched for images, or
|
||||||
/// fetched on demand for files). Keyed by attachment id so the UI can match
|
/// fetched on demand for files). Keyed by `(from, id)`: the id is
|
||||||
/// it to the chat entry.
|
/// attacker-chosen, so a malicious peer can reuse a victim's id — the author
|
||||||
AttachmentReady { id: crate::files::AttachmentId, data: Vec<u8> },
|
/// disambiguates whose bytes these are and stops content aliasing (Tier C
|
||||||
|
/// F-12).
|
||||||
|
AttachmentReady { from: EndpointId, id: crate::files::AttachmentId, data: Vec<u8> },
|
||||||
/// An attachment fetch failed (sender gone, too large, decode error, etc.).
|
/// An attachment fetch failed (sender gone, too large, decode error, etc.).
|
||||||
AttachmentFailed { id: crate::files::AttachmentId, error: String },
|
AttachmentFailed { from: EndpointId, id: crate::files::AttachmentId, error: String },
|
||||||
/// Our own screen share started; the UI flips the Share button to "Stop".
|
/// Our own screen share started; the UI flips the Share button to "Stop".
|
||||||
ScreenShareStarted,
|
ScreenShareStarted,
|
||||||
/// Our own screen share stopped (or failed to start).
|
/// Our own screen share stopped (or failed to start).
|
||||||
|
|||||||
+3
-2
@@ -827,6 +827,7 @@ fn spawn_attachment_fetch(
|
|||||||
if is_image && crate::files::validate_image_bytes(&data).is_none() {
|
if is_image && crate::files::validate_image_bytes(&data).is_none() {
|
||||||
let _ = ui_tx
|
let _ = ui_tx
|
||||||
.send(UiEvent::AttachmentFailed {
|
.send(UiEvent::AttachmentFailed {
|
||||||
|
from,
|
||||||
id: att.id,
|
id: att.id,
|
||||||
error: "received image failed to decode".to_string(),
|
error: "received image failed to decode".to_string(),
|
||||||
})
|
})
|
||||||
@@ -834,12 +835,12 @@ fn spawn_attachment_fetch(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
let _ = ui_tx
|
let _ = ui_tx
|
||||||
.send(UiEvent::AttachmentReady { id: att.id, data })
|
.send(UiEvent::AttachmentReady { from, id: att.id, data })
|
||||||
.await;
|
.await;
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let _ = ui_tx
|
let _ = ui_tx
|
||||||
.send(UiEvent::AttachmentFailed { id: att.id, error: e.to_string() })
|
.send(UiEvent::AttachmentFailed { from, id: att.id, error: e.to_string() })
|
||||||
.await;
|
.await;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user