From f1ae6002ecaaf74e64a41cd0566c6d6ea5b94ef9 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Wed, 29 Jul 2026 15:20:34 +0800 Subject: [PATCH] feat(editor): show authenticated profile avatars --- Cargo.lock | 3 + crates/op-auth-bridge/src/status.rs | 4 +- crates/op-editor-core/src/auth_routes.rs | 16 +- .../op-editor-ui/src/collab_avatar_runtime.rs | 409 +++++++++++------- .../src/collab_avatar_runtime_tests.rs | 255 +++++++++++ .../src/widgets/account_avatar_paint.rs | 154 +++++++ .../src/widgets/account_press_flow.rs | 1 + .../src/widgets/agent_settings_press_flow.rs | 1 + crates/op-editor-ui/src/widgets/mod.rs | 1 + crates/op-editor-ui/src/widgets/top_bar.rs | 3 +- .../op-editor-ui/src/widgets/top_bar_paint.rs | 9 +- .../op-host-desktop/src/collab_avatar_host.rs | 151 +------ crates/op-host-desktop/src/main.rs | 4 + .../src/widget_host/auth_flow.rs | 5 + crates/op-host-services/Cargo.toml | 3 + crates/op-host-services/src/lib.rs | 1 + .../src/profile_avatar_fetch.rs | 280 ++++++++++++ crates/op-host-services/src/provider_dial.rs | 68 ++- .../src/public_https_client.rs | 30 +- crates/op-host-services/src/web_auth.rs | 53 +++ .../src/web_canvas_server/connection.rs | 12 + .../src/web_canvas_server_tests.rs | 34 ++ crates/op-host-web/src/image_decode_queue.rs | 52 ++- crates/op-host-web/src/web_auth_sync.rs | 136 +++++- 24 files changed, 1366 insertions(+), 319 deletions(-) create mode 100644 crates/op-editor-ui/src/collab_avatar_runtime_tests.rs create mode 100644 crates/op-editor-ui/src/widgets/account_avatar_paint.rs create mode 100644 crates/op-host-services/src/profile_avatar_fetch.rs diff --git a/Cargo.lock b/Cargo.lock index b62950f00..afc463c6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3849,8 +3849,10 @@ dependencies = [ "base64", "dirs 5.0.1", "futures", + "getrandom 0.3.4", "github-copilot-sdk", "glam", + "hmac", "jian-ops-schema", "memmap2", "op-acp", @@ -3874,6 +3876,7 @@ dependencies = [ "reqwest 0.12.28", "serde", "serde_json", + "sha2", "skia-safe", "tokio", "zip", diff --git a/crates/op-auth-bridge/src/status.rs b/crates/op-auth-bridge/src/status.rs index 43fcd6755..df70c0621 100644 --- a/crates/op-auth-bridge/src/status.rs +++ b/crates/op-auth-bridge/src/status.rs @@ -73,13 +73,13 @@ mod tests { #[test] fn decodes_signed_in_payload() { - let payload = r#"{"display_name":"Kay","primary_email":"kay@example.com","avatar_url":null,"device_id":"d1"}"#; + let payload = r#"{"display_name":"Kay","primary_email":"kay@example.com","avatar_url":"https://cdn.example/kay.png","device_id":"d1"}"#; assert_eq!( AuthStatus::decode(4, Some(payload)), AuthStatus::SignedIn { display_name: "Kay".to_string(), primary_email: Some("kay@example.com".to_string()), - avatar_url: None, + avatar_url: Some("https://cdn.example/kay.png".to_string()), device_id: "d1".to_string(), } ); diff --git a/crates/op-editor-core/src/auth_routes.rs b/crates/op-editor-core/src/auth_routes.rs index 7c2349023..097231054 100644 --- a/crates/op-editor-core/src/auth_routes.rs +++ b/crates/op-editor-core/src/auth_routes.rs @@ -17,6 +17,13 @@ pub const API_PREFIX: &str = "/api/auth/"; /// `GET` — session/status snapshot (seeds `account_ui_available`). pub const STATUS: &str = "/api/auth/status"; +/// `POST` — bounded current-account avatar bytes from the same-origin daemon. +/// +/// JSON POST is intentional: unlike a GET, a cross-site image element cannot +/// trigger the daemon's upstream fetch without passing the existing +/// same-origin and JSON content-type gates. +pub const AVATAR: &str = "/api/auth/avatar"; + /// `POST` — begin a device-login pairing. The daemon holds the request /// until the pairing's verification URI is known. pub const LOGIN_BEGIN: &str = "/api/auth/login/begin"; @@ -36,7 +43,14 @@ mod tests { #[test] fn api_routes_share_the_gating_prefix() { - for route in [STATUS, LOGIN_BEGIN, LOGIN_STATUS, LOGIN_CANCEL, LOGOUT] { + for route in [ + STATUS, + AVATAR, + LOGIN_BEGIN, + LOGIN_STATUS, + LOGIN_CANCEL, + LOGOUT, + ] { assert!( route.starts_with(API_PREFIX), "{route} outside {API_PREFIX}" diff --git a/crates/op-editor-ui/src/collab_avatar_runtime.rs b/crates/op-editor-ui/src/collab_avatar_runtime.rs index 24512d8a2..845ed87f2 100644 --- a/crates/op-editor-ui/src/collab_avatar_runtime.rs +++ b/crates/op-editor-ui/src/collab_avatar_runtime.rs @@ -1,14 +1,14 @@ -//! Bounded host/widget handoff for verified collaboration avatars. +//! Bounded host/widget handoff for verified profile avatars. //! -//! URLs arrive only from the authenticated collaboration roster and stay in -//! this ephemeral process-local registry; encoded bytes stay in its LRU. -//! Neither enters `EditorState`, document serialization, or debug output. -//! Widgets read only by epoch-local participant key; the desktop host owns -//! HTTPS and the existing image decode workers own rasterization. +//! URLs arrive from authenticated account/collaboration profiles and stay in +//! this ephemeral process-local registry; encoded bytes stay in its LRU. The +//! desktop host owns SSRF-safe HTTPS and the existing image decode workers own +//! rasterization. use std::collections::{HashMap, VecDeque}; use std::fmt; use std::sync::{Arc, Mutex, OnceLock}; +use std::time::{Duration, Instant}; pub const MAX_AVATAR_ENCODED_BYTES: usize = 512 * 1024; pub const MAX_AVATAR_SOURCE_EDGE_PX: u32 = 1_024; @@ -17,9 +17,15 @@ pub const AVATAR_DECODE_EDGE_PX: u32 = 64; const MAX_AVATAR_URL_BYTES: usize = 2_048; const MAX_PARTICIPANT_KEY_BYTES: usize = 256; +const MAX_ACCOUNT_REVISION_BYTES: usize = 128; +const COLLAB_KEY_PREFIX: &str = "collab:"; +const MAX_REGISTRY_KEY_BYTES: usize = MAX_PARTICIPANT_KEY_BYTES + COLLAB_KEY_PREFIX.len(); +const ACCOUNT_AVATAR_KEY: &str = "account:current"; const AVATAR_CACHE_BYTE_BUDGET: usize = 8 * 1024 * 1024; const AVATAR_CACHE_MAX_ENTRIES: usize = 64; const AVATAR_PENDING_CAP: usize = 32; +const ACCOUNT_AVATAR_MAX_RETRIES: u8 = 6; +const ACCOUNT_AVATAR_MAX_RETRY_DELAY: Duration = Duration::from_secs(30); const IMAGE_ID_PREFIX: u64 = 0xa710_0000_0000_0000; /// Opaque request token. Its custom `Debug` deliberately omits the URL and @@ -37,6 +43,15 @@ impl CollabAvatarFetchRequest { pub fn url(&self) -> &str { &self.url } + + /// Whether this request belongs to the locally authenticated account. + /// + /// Remote collaboration participants always occupy the separate + /// `collab:` namespace and therefore cannot opt into account-only host + /// fetch policy. + pub fn is_current_account(&self) -> bool { + self.participant_key == ACCOUNT_AVATAR_KEY + } } impl fmt::Debug for CollabAvatarFetchRequest { @@ -81,6 +96,8 @@ struct AvatarSlot { image_id: u64, state: SlotState, last_used: u64, + failure_count: u8, + retry_at: Option, } struct AvatarRegistry { @@ -155,6 +172,8 @@ impl AvatarRegistry { image_id, state: SlotState::Waiting, last_used: tick, + failure_count: 0, + retry_at: None, }, ); } @@ -196,11 +215,29 @@ impl AvatarRegistry { fn ready(&mut self, participant_key: &str) -> Option { if participant_key.is_empty() - || participant_key.len() > MAX_PARTICIPANT_KEY_BYTES + || participant_key.len() > MAX_REGISTRY_KEY_BYTES || participant_key.chars().any(char::is_control) { return None; } + if participant_key == ACCOUNT_AVATAR_KEY { + let retry_url = self.slots.get_mut(participant_key).and_then(|slot| { + let retry_due = matches!(slot.state, SlotState::Failed) + && slot + .retry_at + .is_some_and(|deadline| Instant::now() >= deadline); + if matches!(slot.state, SlotState::Waiting) || retry_due { + slot.state = SlotState::Waiting; + slot.retry_at = None; + Some(slot.url.clone()) + } else { + None + } + }); + if let Some(url) = retry_url { + let _ = self.lookup(participant_key, &url); + } + } self.tick = self.tick.wrapping_add(1); let tick = self.tick; let slot = self.slots.get_mut(participant_key)?; @@ -269,6 +306,8 @@ impl AvatarRegistry { slot.last_used = self.tick; match encoded { Some(encoded) => { + slot.failure_count = 0; + slot.retry_at = None; self.cached_bytes = self.cached_bytes.saturating_add(encoded.len()); self.encoded.insert(slot.image_id, encoded); slot.state = SlotState::Ready; @@ -279,6 +318,18 @@ impl AvatarRegistry { } None => { slot.state = SlotState::Failed; + if request.participant_key == ACCOUNT_AVATAR_KEY + && slot.failure_count < ACCOUNT_AVATAR_MAX_RETRIES + { + slot.failure_count += 1; + slot.retry_at = Some( + Instant::now() + + account_avatar_retry_delay(slot.failure_count) + .min(ACCOUNT_AVATAR_MAX_RETRY_DELAY), + ); + } else { + slot.retry_at = None; + } true } } @@ -288,16 +339,105 @@ impl AvatarRegistry { self.encoded.get(&image_id).map(Arc::clone) } + fn has_background_work(&self) -> bool { + !self.pending.is_empty() + || self.slots.get(ACCOUNT_AVATAR_KEY).is_some_and(|slot| { + matches!( + slot.state, + SlotState::Waiting | SlotState::Queued | SlotState::InFlight + ) || (matches!(slot.state, SlotState::Failed) && slot.retry_at.is_some()) + }) + } + + fn install_ready( + &mut self, + participant_key: &str, + source_identity: &str, + bytes: Vec, + ) -> bool { + let Some(encoded) = validate_encoded_avatar(bytes) else { + self.remove_slot(participant_key); + return false; + }; + let changed = self + .slots + .get(participant_key) + .is_some_and(|slot| slot.url != source_identity); + if changed { + self.remove_slot(participant_key); + } + self.tick = self.tick.wrapping_add(1); + let tick = self.tick; + if !self.slots.contains_key(participant_key) { + if self.max_entries == 0 { + return false; + } + self.evict_to_entry_limit(self.max_entries.saturating_sub(1)); + let image_id = IMAGE_ID_PREFIX | self.next_image_id; + self.next_image_id = self.next_image_id.wrapping_add(1).max(1); + self.slots.insert( + participant_key.to_string(), + AvatarSlot { + url: source_identity.to_string(), + profile_generation: 1, + image_id, + state: SlotState::Ready, + last_used: tick, + failure_count: 0, + retry_at: None, + }, + ); + } + let slot = self + .slots + .get_mut(participant_key) + .expect("ready slot was inserted above"); + slot.last_used = tick; + slot.state = SlotState::Ready; + slot.failure_count = 0; + slot.retry_at = None; + if let Some(previous) = self.encoded.insert(slot.image_id, encoded.clone()) { + self.cached_bytes = self.cached_bytes.saturating_sub(previous.len()); + } + self.cached_bytes = self.cached_bytes.saturating_add(encoded.len()); + self.evict_over_budget(); + self.slots + .get(participant_key) + .is_some_and(|slot| matches!(slot.state, SlotState::Ready)) + } + fn begin_session_generation(&mut self, generation: u64) -> bool { + let account_request_invalidated = self + .slots + .get(ACCOUNT_AVATAR_KEY) + .is_some_and(|slot| matches!(slot.state, SlotState::Queued | SlotState::InFlight)); let changed = self.session_generation != generation || !self.pending.is_empty() - || !self.slots.is_empty() - || !self.encoded.is_empty(); + || self.slots.keys().any(|key| key != ACCOUNT_AVATAR_KEY) + || account_request_invalidated; self.session_generation = generation; + + let stale_keys: Vec = self + .slots + .keys() + .filter(|key| key.as_str() != ACCOUNT_AVATAR_KEY) + .cloned() + .collect(); + for key in stale_keys { + self.remove_slot(&key); + } self.pending.clear(); - self.slots.clear(); - self.encoded.clear(); - self.cached_bytes = 0; + let account_retry_url = self.slots.get_mut(ACCOUNT_AVATAR_KEY).and_then(|account| { + if matches!(account.state, SlotState::Queued | SlotState::InFlight) { + account.state = SlotState::Waiting; + Some(account.url.clone()) + } else { + None + } + }); + if let Some(url) = account_retry_url { + let _ = self.lookup(ACCOUNT_AVATAR_KEY, &url); + } self.tick = self.tick.wrapping_add(1); // Keep `next_image_id` monotonic: a late decode result may still land // in the backend LRU, and a fresh generation must never reuse its id. @@ -319,8 +459,15 @@ impl AvatarRegistry { let Some(oldest) = self .slots .iter() + .filter(|(key, _)| key.as_str() != ACCOUNT_AVATAR_KEY) .min_by_key(|(_, slot)| slot.last_used) .map(|(key, _)| key.clone()) + .or_else(|| { + self.slots + .iter() + .min_by_key(|(_, slot)| slot.last_used) + .map(|(key, _)| key.clone()) + }) else { break; }; @@ -333,8 +480,15 @@ impl AvatarRegistry { let Some(oldest) = self .slots .iter() + .filter(|(key, _)| key.as_str() != ACCOUNT_AVATAR_KEY) .min_by_key(|(_, slot)| slot.last_used) .map(|(key, _)| key.clone()) + .or_else(|| { + self.slots + .iter() + .min_by_key(|(_, slot)| slot.last_used) + .map(|(key, _)| key.clone()) + }) else { break; }; @@ -355,31 +509,80 @@ fn avatars() -> &'static Mutex { /// epoch-local participant key. Invalid/absent profiles remove any stale /// image for that participant. pub fn register_collab_avatar_url(participant_key: &str, url: Option<&str>) -> bool { + let Some(participant_key) = collab_registry_key(participant_key) else { + return false; + }; let Ok(mut registry) = avatars().lock() else { return false; }; let Some(url) = url else { - registry.remove_slot(participant_key); + registry.remove_slot(&participant_key); return true; }; - if !valid_lookup(participant_key, url) { - registry.remove_slot(participant_key); + if !valid_lookup(&participant_key, url) { + registry.remove_slot(&participant_key); return false; } - let _ = registry.lookup(participant_key, url); + let _ = registry.lookup(&participant_key, url); true } /// Resolve a ready avatar by opaque participant key only. pub fn collab_avatar_image(participant_key: &str) -> Option { - avatars().lock().ok()?.ready(participant_key) + let participant_key = collab_registry_key(participant_key)?; + avatars().lock().ok()?.ready(&participant_key) +} + +/// Register or clear the current authenticated account's profile image. +/// +/// The account lives in a separate namespace from collaboration participant +/// keys, so an untrusted roster key cannot replace the signed-in user's image. +pub fn register_account_avatar_url(url: Option<&str>) -> bool { + let Ok(mut registry) = avatars().lock() else { + return false; + }; + let Some(url) = url else { + registry.remove_slot(ACCOUNT_AVATAR_KEY); + return true; + }; + if !valid_lookup(ACCOUNT_AVATAR_KEY, url) { + registry.remove_slot(ACCOUNT_AVATAR_KEY); + return false; + } + let _ = registry.lookup(ACCOUNT_AVATAR_KEY, url); + true +} + +/// Resolve the current account image after its host fetch has completed. +pub fn account_avatar_image() -> Option { + avatars().lock().ok()?.ready(ACCOUNT_AVATAR_KEY) +} + +/// Install bytes returned by the authenticated serve-web avatar proxy. +/// +/// `revision` is an opaque, URL-derived identity emitted by the daemon. The +/// browser never receives the underlying profile URL. +pub fn install_account_avatar_bytes(revision: &str, bytes: Vec) -> bool { + if revision.is_empty() + || revision.len() > MAX_ACCOUNT_REVISION_BYTES + || !revision + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_')) + { + return false; + } + let source_identity = format!("proxy:{revision}"); + avatars() + .lock() + .map(|mut registry| registry.install_ready(ACCOUNT_AVATAR_KEY, &source_identity, bytes)) + .unwrap_or(false) } /// Begin a process-local avatar epoch for a collaboration runtime. -/// Pending, failed, and ready entries are dropped on every call, including -/// when a newly constructed runtime reuses the same numeric generation. -/// In-flight workers may finish, but their request tokens can no longer -/// complete because opaque image ids are never reused. +/// Collaboration entries are dropped on every call, including when a newly +/// constructed runtime reuses the same numeric generation. The independent +/// current-account slot survives; an in-flight account request is safely +/// requeued under the new generation. pub fn begin_collab_avatar_generation(generation: u64) -> bool { avatars() .lock() @@ -414,13 +617,18 @@ pub fn cached_collab_avatar_bytes(image_id: u64) -> Option> { pub fn has_pending_collab_avatar_requests() -> bool { avatars() .lock() - .map(|registry| !registry.pending.is_empty()) + .map(|registry| registry.has_background_work()) .unwrap_or(false) } +fn account_avatar_retry_delay(failure_count: u8) -> Duration { + let exponent = u32::from(failure_count.saturating_sub(1).min(5)); + Duration::from_secs(1_u64 << exponent) +} + fn valid_lookup(participant_key: &str, url: &str) -> bool { !participant_key.is_empty() - && participant_key.len() <= MAX_PARTICIPANT_KEY_BYTES + && participant_key.len() <= MAX_REGISTRY_KEY_BYTES && !participant_key.chars().any(char::is_control) && !url.is_empty() && url.len() <= MAX_AVATAR_URL_BYTES @@ -428,6 +636,16 @@ fn valid_lookup(participant_key: &str, url: &str) -> bool { && !url.chars().any(char::is_whitespace) } +fn collab_registry_key(participant_key: &str) -> Option { + if participant_key.is_empty() + || participant_key.len() > MAX_PARTICIPANT_KEY_BYTES + || participant_key.chars().any(char::is_control) + { + return None; + } + Some(format!("{COLLAB_KEY_PREFIX}{participant_key}")) +} + fn validate_encoded_avatar(bytes: Vec) -> Option> { if bytes.is_empty() || bytes.len() > MAX_AVATAR_ENCODED_BYTES { return None; @@ -455,146 +673,5 @@ pub(crate) fn lock_collab_avatar_registry_for_tests() -> std::sync::MutexGuard<' } #[cfg(test)] -mod tests { - use super::*; - - fn png_header(width: u32, height: u32, payload_bytes: usize) -> Vec { - let mut bytes = vec![0; payload_bytes.max(24)]; - bytes[..8].copy_from_slice(b"\x89PNG\r\n\x1a\n"); - bytes[12..16].copy_from_slice(b"IHDR"); - bytes[16..20].copy_from_slice(&width.to_be_bytes()); - bytes[20..24].copy_from_slice(&height.to_be_bytes()); - bytes - } - - fn fetch(registry: &mut AvatarRegistry, key: &str, url: &str) -> CollabAvatarFetchRequest { - assert!(registry.lookup(key, url).is_none()); - registry.take_requests(1).pop().expect("request queued") - } - - #[test] - fn success_is_cached_and_failures_keep_the_fallback() { - let mut registry = AvatarRegistry::with_limits(1_024, 4, 4); - let ok = fetch(&mut registry, "p1", "https://cdn.example/a.png"); - assert!(registry.complete(&ok, Some(png_header(16, 16, 48)))); - let image = registry - .lookup("p1", "https://cdn.example/a.png") - .expect("valid response is ready"); - assert_eq!(image.image_id, ok.image_id); - - let failed = fetch(&mut registry, "p2", "https://cdn.example/b.png"); - assert!(registry.complete(&failed, None)); - assert!(registry.lookup("p2", "https://cdn.example/b.png").is_none()); - assert!(registry.take_requests(1).is_empty()); - } - - #[test] - fn encoded_bytes_dimensions_and_queue_are_bounded() { - let mut registry = AvatarRegistry::with_limits(2_048, 64, 1); - let large_body = fetch(&mut registry, "p1", "https://cdn.example/large.png"); - assert!(registry.complete(&large_body, Some(vec![0; MAX_AVATAR_ENCODED_BYTES + 1]))); - assert!(registry - .lookup("p1", "https://cdn.example/large.png") - .is_none()); - - let large_raster = fetch(&mut registry, "p2", "https://cdn.example/wide.png"); - assert!(registry.complete( - &large_raster, - Some(png_header(MAX_AVATAR_SOURCE_EDGE_PX + 1, 1, 24)) - )); - assert!(registry - .lookup("p2", "https://cdn.example/wide.png") - .is_none()); - - assert!(registry.lookup("p3", "https://cdn.example/c.png").is_none()); - assert!(registry.lookup("p4", "https://cdn.example/d.png").is_none()); - assert_eq!(registry.take_requests(8).len(), 1); - } - - #[test] - fn lru_eviction_and_generation_switch_drop_stale_results() { - let mut registry = AvatarRegistry::with_limits(80, 2, 4); - let first = fetch(&mut registry, "p1", "https://cdn.example/one.png"); - assert!(registry.complete(&first, Some(png_header(2, 2, 40)))); - let second = fetch(&mut registry, "p2", "https://cdn.example/two.png"); - assert!(registry.complete(&second, Some(png_header(2, 2, 40)))); - assert!(registry - .lookup("p1", "https://cdn.example/one.png") - .is_some()); - let third = fetch(&mut registry, "p3", "https://cdn.example/three.png"); - assert!(registry.complete(&third, Some(png_header(2, 2, 40)))); - assert!( - !registry.slots.contains_key("p2"), - "least recently used entry is evicted" - ); - - let old = fetch(&mut registry, "p4", "https://cdn.example/old.png"); - assert!(registry - .lookup("p4", "https://cdn.example/new.png") - .is_none()); - let new = registry.take_requests(1).pop().expect("new generation"); - assert!(!registry.complete(&old, Some(png_header(2, 2, 32)))); - assert!(registry - .lookup("p4", "https://cdn.example/new.png") - .is_none()); - assert!(registry.complete(&new, Some(png_header(2, 2, 32)))); - assert_eq!( - registry - .lookup("p4", "https://cdn.example/new.png") - .expect("new generation wins") - .image_id, - new.image_id - ); - } - - #[test] - fn session_generation_rotation_clears_cache_and_rejects_late_workers() { - let mut registry = AvatarRegistry::with_limits(1_024, 4, 4); - assert!(registry.begin_session_generation(7)); - let stale = fetch( - &mut registry, - "same-participant", - "https://cdn.example/same.png", - ); - assert!(registry.begin_session_generation(8)); - assert!(registry.slots.is_empty()); - assert!(registry.pending.is_empty()); - assert!(!registry.complete(&stale, Some(png_header(16, 16, 32)))); - - let current = fetch( - &mut registry, - "same-participant", - "https://cdn.example/same.png", - ); - assert_ne!(stale.image_id, current.image_id); - assert!( - registry.begin_session_generation(8), - "a new runtime may reuse the numeric generation and must still reset" - ); - assert!(!registry.complete(¤t, Some(png_header(16, 16, 32)))); - let refreshed = fetch( - &mut registry, - "same-participant", - "https://cdn.example/same.png", - ); - assert_ne!(current.image_id, refreshed.image_id); - assert!(registry.complete(&refreshed, Some(png_header(16, 16, 32)))); - assert!(registry - .lookup("same-participant", "https://cdn.example/same.png") - .is_some()); - } - - #[test] - fn request_debug_redacts_url_and_identity() { - let mut registry = AvatarRegistry::with_limits(100, 1, 1); - let request = fetch( - &mut registry, - "private-participant", - "https://cdn.example/secret.png", - ); - let debug = format!("{request:?}"); - assert!(!debug.contains("private-participant")); - assert!(!debug.contains("cdn.example")); - assert!(debug.contains("[REDACTED]")); - } -} +#[path = "collab_avatar_runtime_tests.rs"] +mod tests; diff --git a/crates/op-editor-ui/src/collab_avatar_runtime_tests.rs b/crates/op-editor-ui/src/collab_avatar_runtime_tests.rs new file mode 100644 index 000000000..f835de8c4 --- /dev/null +++ b/crates/op-editor-ui/src/collab_avatar_runtime_tests.rs @@ -0,0 +1,255 @@ +use super::*; + +fn png_header(width: u32, height: u32, payload_bytes: usize) -> Vec { + let mut bytes = vec![0; payload_bytes.max(24)]; + bytes[..8].copy_from_slice(b"\x89PNG\r\n\x1a\n"); + bytes[12..16].copy_from_slice(b"IHDR"); + bytes[16..20].copy_from_slice(&width.to_be_bytes()); + bytes[20..24].copy_from_slice(&height.to_be_bytes()); + bytes +} + +fn fetch(registry: &mut AvatarRegistry, key: &str, url: &str) -> CollabAvatarFetchRequest { + assert!(registry.lookup(key, url).is_none()); + registry.take_requests(1).pop().expect("request queued") +} + +#[test] +fn success_is_cached_and_failures_keep_the_fallback() { + let mut registry = AvatarRegistry::with_limits(1_024, 4, 4); + let ok = fetch(&mut registry, "p1", "https://cdn.example/a.png"); + assert!(registry.complete(&ok, Some(png_header(16, 16, 48)))); + let image = registry + .lookup("p1", "https://cdn.example/a.png") + .expect("valid response is ready"); + assert_eq!(image.image_id, ok.image_id); + + let failed = fetch(&mut registry, "p2", "https://cdn.example/b.png"); + assert!(registry.complete(&failed, None)); + assert!(registry.lookup("p2", "https://cdn.example/b.png").is_none()); + assert!(registry.take_requests(1).is_empty()); +} + +#[test] +fn encoded_bytes_dimensions_and_queue_are_bounded() { + let mut registry = AvatarRegistry::with_limits(2_048, 64, 1); + let large_body = fetch(&mut registry, "p1", "https://cdn.example/large.png"); + assert!(registry.complete(&large_body, Some(vec![0; MAX_AVATAR_ENCODED_BYTES + 1]))); + assert!(registry + .lookup("p1", "https://cdn.example/large.png") + .is_none()); + + let large_raster = fetch(&mut registry, "p2", "https://cdn.example/wide.png"); + assert!(registry.complete( + &large_raster, + Some(png_header(MAX_AVATAR_SOURCE_EDGE_PX + 1, 1, 24)) + )); + assert!(registry + .lookup("p2", "https://cdn.example/wide.png") + .is_none()); + + assert!(registry.lookup("p3", "https://cdn.example/c.png").is_none()); + assert!(registry.lookup("p4", "https://cdn.example/d.png").is_none()); + assert_eq!(registry.take_requests(8).len(), 1); +} + +#[test] +fn lru_eviction_and_generation_switch_drop_stale_results() { + let mut registry = AvatarRegistry::with_limits(80, 2, 4); + let first = fetch(&mut registry, "p1", "https://cdn.example/one.png"); + assert!(registry.complete(&first, Some(png_header(2, 2, 40)))); + let second = fetch(&mut registry, "p2", "https://cdn.example/two.png"); + assert!(registry.complete(&second, Some(png_header(2, 2, 40)))); + assert!(registry + .lookup("p1", "https://cdn.example/one.png") + .is_some()); + let third = fetch(&mut registry, "p3", "https://cdn.example/three.png"); + assert!(registry.complete(&third, Some(png_header(2, 2, 40)))); + assert!( + !registry.slots.contains_key("p2"), + "least recently used entry is evicted" + ); + + let old = fetch(&mut registry, "p4", "https://cdn.example/old.png"); + assert!(registry + .lookup("p4", "https://cdn.example/new.png") + .is_none()); + let new = registry.take_requests(1).pop().expect("new generation"); + assert!(!registry.complete(&old, Some(png_header(2, 2, 32)))); + assert!(registry + .lookup("p4", "https://cdn.example/new.png") + .is_none()); + assert!(registry.complete(&new, Some(png_header(2, 2, 32)))); + assert_eq!( + registry + .lookup("p4", "https://cdn.example/new.png") + .expect("new generation wins") + .image_id, + new.image_id + ); +} + +#[test] +fn session_generation_rotation_clears_cache_and_rejects_late_workers() { + let mut registry = AvatarRegistry::with_limits(1_024, 4, 4); + assert!(registry.begin_session_generation(7)); + let stale = fetch( + &mut registry, + "same-participant", + "https://cdn.example/same.png", + ); + assert!(registry.begin_session_generation(8)); + assert!(registry.slots.is_empty()); + assert!(registry.pending.is_empty()); + assert!(!registry.complete(&stale, Some(png_header(16, 16, 32)))); + + let current = fetch( + &mut registry, + "same-participant", + "https://cdn.example/same.png", + ); + assert_ne!(stale.image_id, current.image_id); + assert!( + registry.begin_session_generation(8), + "a new runtime may reuse the numeric generation and must still reset" + ); + assert!(!registry.complete(¤t, Some(png_header(16, 16, 32)))); + let refreshed = fetch( + &mut registry, + "same-participant", + "https://cdn.example/same.png", + ); + assert_ne!(current.image_id, refreshed.image_id); + assert!(registry.complete(&refreshed, Some(png_header(16, 16, 32)))); + assert!(registry + .lookup("same-participant", "https://cdn.example/same.png") + .is_some()); +} + +#[test] +fn collaboration_generation_preserves_and_requeues_the_account_slot() { + let mut registry = AvatarRegistry::with_limits(1_024, 4, 4); + let account = fetch( + &mut registry, + ACCOUNT_AVATAR_KEY, + "https://cdn.example/account.png", + ); + assert!(registry.begin_session_generation(7)); + assert!(!registry.complete(&account, Some(png_header(16, 16, 32)))); + let retried = registry + .take_requests(1) + .pop() + .expect("account request requeued"); + assert_eq!(retried.image_id, account.image_id); + assert!(registry.complete(&retried, Some(png_header(16, 16, 32)))); + + let collab = fetch( + &mut registry, + "collab:participant", + "https://cdn.example/participant.png", + ); + assert!(registry.complete(&collab, Some(png_header(16, 16, 32)))); + assert!(registry.begin_session_generation(8)); + assert!(registry.ready(ACCOUNT_AVATAR_KEY).is_some()); + assert!(!registry.slots.contains_key("collab:participant")); +} + +#[test] +fn failed_account_fetch_retries_with_backoff_and_keeps_the_fallback() { + let mut registry = AvatarRegistry::with_limits(1_024, 4, 4); + let failed = fetch( + &mut registry, + ACCOUNT_AVATAR_KEY, + "https://cdn.example/account.png", + ); + assert!(registry.complete(&failed, None)); + assert!(registry.ready(ACCOUNT_AVATAR_KEY).is_none()); + assert!(registry.has_background_work()); + assert!(registry.pending.is_empty()); + + registry + .slots + .get_mut(ACCOUNT_AVATAR_KEY) + .expect("account slot") + .retry_at = Some(Instant::now()); + assert!(registry.ready(ACCOUNT_AVATAR_KEY).is_none()); + let retried = registry + .take_requests(1) + .pop() + .expect("due account retry queued"); + assert_eq!(retried.image_id, failed.image_id); + assert!(registry.complete(&retried, Some(png_header(16, 16, 32)))); + assert!(registry.ready(ACCOUNT_AVATAR_KEY).is_some()); +} + +#[test] +fn account_and_adversarial_participant_keys_cannot_collide() { + let _guard = lock_collab_avatar_registry_for_tests(); + assert!(register_account_avatar_url(Some( + "https://cdn.example/account.png" + ))); + assert!(register_collab_avatar_url( + ACCOUNT_AVATAR_KEY, + Some("https://cdn.example/participant.png") + )); + let requests = take_collab_avatar_requests(2); + assert_eq!(requests.len(), 2); + assert_eq!( + requests + .iter() + .filter(|request| request.is_current_account()) + .count(), + 1 + ); + for request in &requests { + assert!(complete_collab_avatar_request( + request, + Some(png_header(16, 16, 32)) + )); + } + let account = account_avatar_image().expect("account image"); + let participant = collab_avatar_image(ACCOUNT_AVATAR_KEY).expect("participant image"); + assert_ne!(account.image_id, participant.image_id); +} + +#[test] +fn proxy_install_replaces_only_on_a_valid_new_revision() { + let _guard = lock_collab_avatar_registry_for_tests(); + assert!(install_account_avatar_bytes( + "revision-one", + png_header(16, 16, 32) + )); + let first = account_avatar_image().expect("first proxy image"); + assert!(!install_account_avatar_bytes( + "../unsafe", + png_header(16, 16, 32) + )); + assert_eq!( + account_avatar_image() + .expect("invalid revision keeps current") + .image_id, + first.image_id + ); + assert!(install_account_avatar_bytes( + "revision-two", + png_header(16, 16, 32) + )); + assert_ne!( + account_avatar_image().expect("new proxy image").image_id, + first.image_id + ); +} + +#[test] +fn request_debug_redacts_url_and_identity() { + let mut registry = AvatarRegistry::with_limits(100, 1, 1); + let request = fetch( + &mut registry, + "private-participant", + "https://cdn.example/secret.png", + ); + let debug = format!("{request:?}"); + assert!(!debug.contains("private-participant")); + assert!(!debug.contains("cdn.example")); + assert!(debug.contains("[REDACTED]")); +} diff --git a/crates/op-editor-ui/src/widgets/account_avatar_paint.rs b/crates/op-editor-ui/src/widgets/account_avatar_paint.rs new file mode 100644 index 000000000..2296fd9ae --- /dev/null +++ b/crates/op-editor-ui/src/widgets/account_avatar_paint.rs @@ -0,0 +1,154 @@ +//! Current-account avatar overlay shared by TopBar paint. + +use crate::collab_avatar_runtime::{account_avatar_image, AVATAR_DECODE_EDGE_PX}; +use crate::widgets::canvas_viewport_image::note_pending_decode; +use crate::widgets::PaintCx; +use crate::{ImageDrawMode, Rect}; +use op_editor_core::AccountState; + +/// Overlay a ready account image on top of the caller's initials fallback. +/// +/// Fetch and decode misses are recorded only; paint never blocks. Returning +/// `false` means the initials remain the visible fallback. +pub(super) fn paint_account_avatar_image( + cx: &mut PaintCx<'_>, + account: &AccountState, + rect: Rect, +) -> bool { + if matches!(account, AccountState::Anonymous) { + return false; + } + let Some(image) = account_avatar_image() else { + return false; + }; + let sharp = cx.backend.image_decoded( + image.image_id, + image.encoded.as_ref(), + AVATAR_DECODE_EDGE_PX, + ); + if !sharp { + note_pending_decode(image.image_id, AVATAR_DECODE_EDGE_PX); + } + if !sharp && !cx.backend.image_resident(image.image_id) { + return false; + } + + let radius = rect.size.x.min(rect.size.y) / 2.0; + cx.backend.save(); + cx.backend.clip_round_rect(rect, radius); + cx.backend.draw_image_with_mode( + rect, + image.image_id, + image.encoded.as_ref(), + ImageDrawMode::Crop, + ); + cx.backend.restore(); + true +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::collab_avatar_runtime::{ + complete_collab_avatar_request, lock_collab_avatar_registry_for_tests, + register_account_avatar_url, take_collab_avatar_requests, + }; + use crate::{Color, Point2D, RenderBackend, TextLayout}; + + #[derive(Default)] + struct AvatarBackend { + image_ready: bool, + image_draws: usize, + } + + impl RenderBackend for AvatarBackend { + fn begin_frame(&mut self) {} + fn end_frame(&mut self) {} + fn fill_rect(&mut self, _: Rect, _: Color) {} + fn stroke_rect(&mut self, _: Rect, _: Color, _: f32) {} + fn draw_text(&mut self, _: &TextLayout, _: Point2D) {} + fn clip_rect(&mut self, _: Rect) {} + fn stroke_line(&mut self, _: Point2D, _: Point2D, _: Color, _: f32) {} + fn fill_round_rect(&mut self, _: Rect, _: f32, _: Color) {} + fn stroke_round_rect(&mut self, _: Rect, _: f32, _: Color, _: f32) {} + fn stroke_svg_path(&mut self, _: &str, _: Point2D, _: f32, _: Color, _: f32) {} + fn image_decoded(&mut self, _: u64, _: &[u8], _: u32) -> bool { + self.image_ready + } + fn image_resident(&mut self, _: u64) -> bool { + self.image_ready + } + fn draw_image_with_mode(&mut self, _: Rect, _: u64, _: &[u8], mode: ImageDrawMode) { + assert_eq!(mode, ImageDrawMode::Crop); + self.image_draws += 1; + } + fn save(&mut self) {} + fn restore(&mut self) {} + fn translate(&mut self, _: Point2D) {} + fn resize(&mut self, _: u32, _: u32) {} + fn dpi_scale(&self) -> f32 { + 1.0 + } + } + + fn account() -> AccountState { + AccountState::SignedIn { + display_name: "Kayshen".into(), + handle: "Kayshen".into(), + } + } + + fn png_header() -> Vec { + let mut bytes = vec![0; 32]; + bytes[..8].copy_from_slice(b"\x89PNG\r\n\x1a\n"); + bytes[12..16].copy_from_slice(b"IHDR"); + bytes[16..20].copy_from_slice(&16_u32.to_be_bytes()); + bytes[20..24].copy_from_slice(&16_u32.to_be_bytes()); + bytes + } + + #[test] + fn ready_profile_image_overlays_the_initials_fallback() { + let _guard = lock_collab_avatar_registry_for_tests(); + assert!(register_account_avatar_url(Some( + "https://cdn.example/account.png" + ))); + let account = account(); + let rect = Rect::xywh(0.0, 0.0, 20.0, 20.0); + let mut backend = AvatarBackend { + image_ready: true, + ..Default::default() + }; + let mut cx = PaintCx { + backend: &mut backend, + }; + + assert!(!paint_account_avatar_image(&mut cx, &account, rect)); + let request = take_collab_avatar_requests(1).pop().unwrap(); + assert!(complete_collab_avatar_request(&request, Some(png_header()))); + assert!(paint_account_avatar_image(&mut cx, &account, rect)); + assert_eq!(backend.image_draws, 1); + } + + #[test] + fn absent_or_unsafe_url_keeps_the_initials_fallback() { + let _guard = lock_collab_avatar_registry_for_tests(); + let rect = Rect::xywh(0.0, 0.0, 20.0, 20.0); + let mut backend = AvatarBackend::default(); + let mut cx = PaintCx { + backend: &mut backend, + }; + + assert!(!paint_account_avatar_image( + &mut cx, + &AccountState::Anonymous, + rect + )); + assert!(!register_account_avatar_url(Some( + "http://127.0.0.1/avatar.png" + ))); + assert!(!paint_account_avatar_image(&mut cx, &account(), rect)); + assert!(take_collab_avatar_requests(1).is_empty()); + assert_eq!(backend.image_draws, 0); + } +} diff --git a/crates/op-editor-ui/src/widgets/account_press_flow.rs b/crates/op-editor-ui/src/widgets/account_press_flow.rs index aa64bb39c..50b0df98d 100644 --- a/crates/op-editor-ui/src/widgets/account_press_flow.rs +++ b/crates/op-editor-ui/src/widgets/account_press_flow.rs @@ -126,6 +126,7 @@ pub fn press_account_menu( Some(AccountMenuRow::SignOut) => { close_account_menu(state); state.editor_ui.account = AccountState::Anonymous; + let _ = crate::collab_avatar_runtime::register_account_avatar_url(None); AccountMenuPress::SignOut } None => { diff --git a/crates/op-editor-ui/src/widgets/agent_settings_press_flow.rs b/crates/op-editor-ui/src/widgets/agent_settings_press_flow.rs index fd7b826d8..b8b14ccf0 100644 --- a/crates/op-editor-ui/src/widgets/agent_settings_press_flow.rs +++ b/crates/op-editor-ui/src/widgets/agent_settings_press_flow.rs @@ -237,6 +237,7 @@ pub fn apply_agent_settings_hit( } AgentSettingsHit::SignOutAccount => { state.editor_ui.account = AccountState::Anonymous; + let _ = crate::collab_avatar_runtime::register_account_avatar_url(None); SettingsPressOutcome::effect(SettingsPress::SignOut) } AgentSettingsHit::FocusMcpPort => { diff --git a/crates/op-editor-ui/src/widgets/mod.rs b/crates/op-editor-ui/src/widgets/mod.rs index a42afa45c..e39074816 100644 --- a/crates/op-editor-ui/src/widgets/mod.rs +++ b/crates/op-editor-ui/src/widgets/mod.rs @@ -197,6 +197,7 @@ mod icons_data; pub mod brand_icons; // Step 4 — extra editor-chrome widgets (TS app parity). +mod account_avatar_paint; pub mod account_menu; pub mod account_press_flow; pub mod agent_settings_account; diff --git a/crates/op-editor-ui/src/widgets/top_bar.rs b/crates/op-editor-ui/src/widgets/top_bar.rs index e35120cd0..6efa68f15 100644 --- a/crates/op-editor-ui/src/widgets/top_bar.rs +++ b/crates/op-editor-ui/src/widgets/top_bar.rs @@ -155,7 +155,8 @@ pub struct TopBar { /// `hit_test` falls back to a char-count estimate. pub chip_text_w: Option, /// Sign-in state — drives the avatar button's glyph (generic user - /// outline when signed out, an initial-letter circle when signed in). + /// outline when signed out, profile image with an initial fallback when + /// signed in). pub account: op_editor_core::AccountState, /// Whether the avatar button paints at all. Follows the host-set /// runtime release gate (`EditorUiState::account_ui_available`): diff --git a/crates/op-editor-ui/src/widgets/top_bar_paint.rs b/crates/op-editor-ui/src/widgets/top_bar_paint.rs index 5deb90ab7..4522d6deb 100644 --- a/crates/op-editor-ui/src/widgets/top_bar_paint.rs +++ b/crates/op-editor-ui/src/widgets/top_bar_paint.rs @@ -557,8 +557,8 @@ fn paint_collaboration_chip( } /// User-avatar button: a generic outline glyph when signed out, or a -/// filled initial-letter circle when signed in. Shares the same -/// hover/press ghost background as its icon-button siblings (Sun / +/// profile image with an initial-letter fallback when signed in. Shares the +/// same hover/press ghost background as its icon-button siblings (Sun / /// Globe / Maximize) so it reads consistently in the chrome row. pub(super) fn paint_account_button( cx: &mut PaintCx<'_>, @@ -611,6 +611,11 @@ pub(super) fn paint_account_button( center_y + 4.0, ), ); + crate::widgets::account_avatar_paint::paint_account_avatar_image( + cx, + account, + avatar_rect, + ); } } } diff --git a/crates/op-host-desktop/src/collab_avatar_host.rs b/crates/op-host-desktop/src/collab_avatar_host.rs index b91844104..066b1b79f 100644 --- a/crates/op-host-desktop/src/collab_avatar_host.rs +++ b/crates/op-host-desktop/src/collab_avatar_host.rs @@ -2,32 +2,19 @@ use std::sync::mpsc::{self, Receiver, TryRecvError}; use std::sync::Arc; -use std::time::Duration; use op_editor_ui::collab_avatar_runtime::{ complete_collab_avatar_request, take_collab_avatar_requests, CollabAvatarFetchRequest, - MAX_AVATAR_ENCODED_BYTES, }; -use reqwest::header::{ACCEPT, LOCATION}; +use op_host_services::profile_avatar_fetch::{ + fetch_account_avatar_blocking, fetch_profile_avatar_blocking, + ProfileAvatarFetchError as AvatarFetchError, +}; const MAX_CONCURRENT_FETCHES: usize = 3; -const MAX_REDIRECTS: usize = 3; -const REQUEST_TIMEOUT: Duration = Duration::from_secs(5); -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum AvatarFetchError { - UrlNotAllowed, - DialRejected, - RequestFailed, - TimedOut, - HttpStatus, - RedirectInvalid, - TooManyRedirects, - TooLarge, - EmptyBody, -} - -type Fetcher = Arc Result, AvatarFetchError> + Send + Sync>; +type Fetcher = + Arc Result, AvatarFetchError> + Send + Sync>; struct FetchJob { request: CollabAvatarFetchRequest, @@ -52,7 +39,13 @@ pub(crate) fn lock_avatar_test_registry() -> std::sync::MutexGuard<'static, ()> impl CollabAvatarHost { pub(crate) fn new() -> Self { - Self::with_fetcher(Arc::new(fetch_avatar_blocking)) + Self::with_fetcher(Arc::new(|request| { + if request.is_current_account() { + fetch_account_avatar_blocking(request.url()) + } else { + fetch_profile_avatar_blocking(request.url()) + } + })) } fn with_fetcher(fetcher: Fetcher) -> Self { @@ -106,112 +99,16 @@ impl CollabAvatarHost { fn spawn_fetch(request: CollabAvatarFetchRequest, fetcher: Fetcher) -> Option { let (tx, rx) = mpsc::channel(); - let url = request.url().to_string(); + let worker_request = request.clone(); std::thread::Builder::new() .name("op-collab-avatar".into()) .spawn(move || { - let _ = tx.send(fetcher(&url)); + let _ = tx.send(fetcher(&worker_request)); }) .ok()?; Some(FetchJob { request, rx }) } -fn fetch_avatar_blocking(url: &str) -> Result, AvatarFetchError> { - op_host_services::chat_runtime::block_on_anywhere(fetch_avatar(url)) -} - -async fn fetch_avatar(url: &str) -> Result, AvatarFetchError> { - let mut url = parse_avatar_url(url)?; - for redirect_count in 0..=MAX_REDIRECTS { - // `public_https_client` resolves + screens every address, disables - // proxies, and pins the socket while the request URL retains the - // original hostname for TLS certificate verification and SNI. - let client = with_timeout( - REQUEST_TIMEOUT, - op_host_services::public_https_client::public_https_client(&url), - ) - .await? - .map_err(|_| AvatarFetchError::DialRejected)?; - let mut response = client - .get(url.clone()) - .header(ACCEPT, "image/webp,image/png,image/jpeg,image/gif") - .timeout(REQUEST_TIMEOUT) - .send() - .await - .map_err(|_| AvatarFetchError::RequestFailed)?; - - if response.status().is_redirection() { - if redirect_count == MAX_REDIRECTS { - return Err(AvatarFetchError::TooManyRedirects); - } - let location = response - .headers() - .get(LOCATION) - .and_then(|value| value.to_str().ok()) - .ok_or(AvatarFetchError::RedirectInvalid)?; - url = parse_avatar_url( - url.join(location) - .map_err(|_| AvatarFetchError::RedirectInvalid)? - .as_str(), - )?; - continue; - } - if !response.status().is_success() { - return Err(AvatarFetchError::HttpStatus); - } - if response - .content_length() - .is_some_and(|length| length > MAX_AVATAR_ENCODED_BYTES as u64) - { - return Err(AvatarFetchError::TooLarge); - } - let mut bytes = Vec::with_capacity( - response - .content_length() - .unwrap_or(0) - .min(MAX_AVATAR_ENCODED_BYTES as u64) as usize, - ); - while let Some(chunk) = response - .chunk() - .await - .map_err(|_| AvatarFetchError::RequestFailed)? - { - if bytes.len().saturating_add(chunk.len()) > MAX_AVATAR_ENCODED_BYTES { - return Err(AvatarFetchError::TooLarge); - } - bytes.extend_from_slice(&chunk); - } - if bytes.is_empty() { - return Err(AvatarFetchError::EmptyBody); - } - return Ok(bytes); - } - Err(AvatarFetchError::TooManyRedirects) -} - -async fn with_timeout(duration: Duration, future: F) -> Result -where - F: std::future::Future, -{ - tokio::time::timeout(duration, future) - .await - .map_err(|_| AvatarFetchError::TimedOut) -} - -fn parse_avatar_url(value: &str) -> Result { - let url = reqwest::Url::parse(value).map_err(|_| AvatarFetchError::UrlNotAllowed)?; - if url.scheme() != "https" - || url.host_str().is_none() - || !url.username().is_empty() - || url.password().is_some() - || url.fragment().is_some() - || url.port() == Some(0) - { - return Err(AvatarFetchError::UrlNotAllowed); - } - Ok(url) -} - #[cfg(test)] mod tests { use super::*; @@ -220,7 +117,7 @@ mod tests { register_collab_avatar_url, take_collab_avatar_requests, }; use std::sync::atomic::{AtomicUsize, Ordering}; - use std::time::Instant; + use std::time::{Duration, Instant}; fn png_header() -> Vec { let mut bytes = vec![0; 32]; @@ -318,7 +215,10 @@ mod tests { "https://cdn.example/avatar.png#fragment", "https://cdn.example:0/avatar.png", ] { - assert_eq!(parse_avatar_url(url), Err(AvatarFetchError::UrlNotAllowed)); + assert_eq!( + fetch_profile_avatar_blocking(url), + Err(AvatarFetchError::UrlNotAllowed) + ); } for url in [ "https://127.0.0.1/avatar.png", @@ -328,19 +228,10 @@ mod tests { "https://[fe80::1]/avatar.png", ] { assert_eq!( - fetch_avatar_blocking(url), + fetch_profile_avatar_blocking(url), Err(AvatarFetchError::DialRejected), "{url}" ); } } - - #[test] - fn resolver_deadline_cancels_a_hanging_dial_future() { - let result = op_host_services::chat_runtime::block_on_anywhere(with_timeout( - Duration::from_millis(1), - std::future::pending::<()>(), - )); - assert_eq!(result, Err(AvatarFetchError::TimedOut)); - } } diff --git a/crates/op-host-desktop/src/main.rs b/crates/op-host-desktop/src/main.rs index 7e2552a77..c134b050a 100644 --- a/crates/op-host-desktop/src/main.rs +++ b/crates/op-host-desktop/src/main.rs @@ -480,9 +480,13 @@ fn init_auth_runtime(host: &mut WidgetHostNative) { if let op_auth_bridge::AuthStatus::SignedIn { display_name, primary_email, + avatar_url, .. } = op_auth_bridge::poll(op_auth_bridge::SESSION_HANDLE) { + let _ = op_editor_ui::collab_avatar_runtime::register_account_avatar_url( + avatar_url.as_deref(), + ); host.editor_state_mut().editor_ui.account = op_editor_core::AccountState::SignedIn { handle: primary_email.unwrap_or_else(|| display_name.clone()), diff --git a/crates/op-host-native/src/widget_host/auth_flow.rs b/crates/op-host-native/src/widget_host/auth_flow.rs index 2d8498559..e6a133cab 100644 --- a/crates/op-host-native/src/widget_host/auth_flow.rs +++ b/crates/op-host-native/src/widget_host/auth_flow.rs @@ -77,8 +77,12 @@ impl WidgetHostNative { AuthStatus::SignedIn { display_name, primary_email, + avatar_url, .. } => { + let _ = op_editor_ui::collab_avatar_runtime::register_account_avatar_url( + avatar_url.as_deref(), + ); ui.account = AccountState::SignedIn { handle: primary_email.unwrap_or_else(|| display_name.clone()), display_name, @@ -127,6 +131,7 @@ impl WidgetHostNative { AuthStatus::Idle ) { self.editor_state.editor_ui.account = AccountState::Anonymous; + let _ = op_editor_ui::collab_avatar_runtime::register_account_avatar_url(None); self.mark_dirty(); return true; } diff --git a/crates/op-host-services/Cargo.toml b/crates/op-host-services/Cargo.toml index e128e229c..9160a83f4 100644 --- a/crates/op-host-services/Cargo.toml +++ b/crates/op-host-services/Cargo.toml @@ -69,6 +69,9 @@ async-trait = "0.1" serde = { workspace = true } serde_json = { workspace = true } base64 = "0.22" +getrandom = "0.3" +hmac = "0.12" +sha2 = "0.10" zip = "2" memmap2 = "0.9" glam = { version = "0.29", default-features = false, features = ["std"] } diff --git a/crates/op-host-services/src/lib.rs b/crates/op-host-services/src/lib.rs index 8d7ac36d7..92995021a 100644 --- a/crates/op-host-services/src/lib.rs +++ b/crates/op-host-services/src/lib.rs @@ -74,6 +74,7 @@ pub mod mcp_serve; pub mod model_discovery; mod model_probe; pub mod pre_validator; +pub mod profile_avatar_fetch; mod provider_dial; pub mod provider_probe; pub mod provider_probe_host; diff --git a/crates/op-host-services/src/profile_avatar_fetch.rs b/crates/op-host-services/src/profile_avatar_fetch.rs new file mode 100644 index 000000000..c12cfe854 --- /dev/null +++ b/crates/op-host-services/src/profile_avatar_fetch.rs @@ -0,0 +1,280 @@ +//! SSRF-safe fetch and validation for authenticated profile avatars. + +use hmac::{Hmac, Mac}; +use op_editor_ui::collab_avatar_runtime::{ + MAX_AVATAR_ENCODED_BYTES, MAX_AVATAR_SOURCE_EDGE_PX, MAX_AVATAR_SOURCE_PIXELS, +}; +use reqwest::header::{ACCEPT, LOCATION}; +use sha2::Sha256; +use std::fmt; +use std::sync::OnceLock; +use std::time::Duration; + +const MAX_AVATAR_URL_BYTES: usize = 2_048; +const MAX_REDIRECTS: usize = 3; +const REQUEST_TIMEOUT: Duration = Duration::from_secs(5); +static AVATAR_REVISION_KEY: OnceLock> = OnceLock::new(); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProfileAvatarFetchError { + UrlNotAllowed, + DialRejected, + RequestFailed, + TimedOut, + HttpStatus, + RedirectInvalid, + TooManyRedirects, + TooLarge, + EmptyBody, + InvalidImage, + RevisionUnavailable, +} + +impl fmt::Display for ProfileAvatarFetchError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(match self { + Self::UrlNotAllowed => "avatar URL is not allowed", + Self::DialRejected => "avatar host is not publicly routable", + Self::RequestFailed => "avatar request failed", + Self::TimedOut => "avatar request timed out", + Self::HttpStatus => "avatar server returned an error status", + Self::RedirectInvalid => "avatar redirect is invalid", + Self::TooManyRedirects => "avatar request redirected too many times", + Self::TooLarge => "avatar image is too large", + Self::EmptyBody => "avatar response is empty", + Self::InvalidImage => "avatar response is not a supported image", + Self::RevisionUnavailable => "avatar revision key is unavailable", + }) + } +} + +impl std::error::Error for ProfileAvatarFetchError {} + +/// Stable opaque identity for one validated avatar URL. The raw URL may carry +/// signed CDN query parameters and must not be returned to the browser. +pub fn profile_avatar_revision(url: &str) -> Result { + let url = parse_avatar_url(url)?; + let mut mac = Hmac::::new_from_slice(avatar_revision_key()?) + .map_err(|_| ProfileAvatarFetchError::RevisionUnavailable)?; + mac.update(url.as_str().as_bytes()); + Ok(format!("{:x}", mac.finalize().into_bytes())) +} + +/// Blocking entry point for desktop workers and serve-web connection threads. +pub fn fetch_profile_avatar_blocking(url: &str) -> Result, ProfileAvatarFetchError> { + crate::chat_runtime::block_on_anywhere(fetch_profile_avatar(url, false)) +} + +/// Blocking fetch for the locally authenticated account's profile image. +/// +/// Unlike remote collaboration profiles, this source may traverse a +/// Clash-style fake-IP TUN when the hostname resolves exclusively into RFC +/// 2544 space. +pub fn fetch_account_avatar_blocking(url: &str) -> Result, ProfileAvatarFetchError> { + crate::chat_runtime::block_on_anywhere(fetch_profile_avatar(url, true)) +} + +async fn fetch_profile_avatar( + url: &str, + allow_account_tunnel: bool, +) -> Result, ProfileAvatarFetchError> { + let mut url = parse_avatar_url(url)?; + for redirect_count in 0..=MAX_REDIRECTS { + let client = if allow_account_tunnel { + with_timeout( + REQUEST_TIMEOUT, + crate::public_https_client::tunnel_compatible_account_asset_client(&url), + ) + .await? + } else { + with_timeout( + REQUEST_TIMEOUT, + crate::public_https_client::public_https_client(&url), + ) + .await? + } + .map_err(|_| ProfileAvatarFetchError::DialRejected)?; + let mut response = client + .get(url.clone()) + .header(ACCEPT, "image/webp,image/png,image/jpeg,image/gif") + .timeout(REQUEST_TIMEOUT) + .send() + .await + .map_err(|_| ProfileAvatarFetchError::RequestFailed)?; + + if response.status().is_redirection() { + if redirect_count == MAX_REDIRECTS { + return Err(ProfileAvatarFetchError::TooManyRedirects); + } + let location = response + .headers() + .get(LOCATION) + .and_then(|value| value.to_str().ok()) + .ok_or(ProfileAvatarFetchError::RedirectInvalid)?; + url = parse_avatar_url( + url.join(location) + .map_err(|_| ProfileAvatarFetchError::RedirectInvalid)? + .as_str(), + )?; + continue; + } + if !response.status().is_success() { + return Err(ProfileAvatarFetchError::HttpStatus); + } + if response + .content_length() + .is_some_and(|length| length > MAX_AVATAR_ENCODED_BYTES as u64) + { + return Err(ProfileAvatarFetchError::TooLarge); + } + let mut bytes = Vec::with_capacity( + response + .content_length() + .unwrap_or(0) + .min(MAX_AVATAR_ENCODED_BYTES as u64) as usize, + ); + while let Some(chunk) = response + .chunk() + .await + .map_err(|_| ProfileAvatarFetchError::RequestFailed)? + { + if bytes.len().saturating_add(chunk.len()) > MAX_AVATAR_ENCODED_BYTES { + return Err(ProfileAvatarFetchError::TooLarge); + } + bytes.extend_from_slice(&chunk); + } + validate_profile_avatar_bytes(&bytes)?; + return Ok(bytes); + } + Err(ProfileAvatarFetchError::TooManyRedirects) +} + +fn validate_profile_avatar_bytes(bytes: &[u8]) -> Result<(), ProfileAvatarFetchError> { + if bytes.is_empty() { + return Err(ProfileAvatarFetchError::EmptyBody); + } + if bytes.len() > MAX_AVATAR_ENCODED_BYTES { + return Err(ProfileAvatarFetchError::TooLarge); + } + let (width, height) = op_editor_ui::image_runtime::encoded_image_dimensions(bytes) + .ok_or(ProfileAvatarFetchError::InvalidImage)?; + if width > MAX_AVATAR_SOURCE_EDGE_PX + || height > MAX_AVATAR_SOURCE_EDGE_PX + || u64::from(width) * u64::from(height) > MAX_AVATAR_SOURCE_PIXELS + { + return Err(ProfileAvatarFetchError::TooLarge); + } + Ok(()) +} + +async fn with_timeout(duration: Duration, future: F) -> Result +where + F: std::future::Future, +{ + tokio::time::timeout(duration, future) + .await + .map_err(|_| ProfileAvatarFetchError::TimedOut) +} + +fn parse_avatar_url(value: &str) -> Result { + if value.is_empty() + || value.len() > MAX_AVATAR_URL_BYTES + || value.chars().any(char::is_whitespace) + { + return Err(ProfileAvatarFetchError::UrlNotAllowed); + } + let url = reqwest::Url::parse(value).map_err(|_| ProfileAvatarFetchError::UrlNotAllowed)?; + if url.scheme() != "https" + || url.host_str().is_none() + || !url.username().is_empty() + || url.password().is_some() + || url.fragment().is_some() + || url.port() == Some(0) + { + return Err(ProfileAvatarFetchError::UrlNotAllowed); + } + Ok(url) +} + +fn avatar_revision_key() -> Result<&'static [u8; 32], ProfileAvatarFetchError> { + AVATAR_REVISION_KEY + .get_or_init(|| { + let mut key = [0_u8; 32]; + getrandom::fill(&mut key).ok().map(|()| key) + }) + .as_ref() + .ok_or(ProfileAvatarFetchError::RevisionUnavailable) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn png_header(width: u32, height: u32) -> Vec { + let mut bytes = vec![0; 32]; + bytes[..8].copy_from_slice(b"\x89PNG\r\n\x1a\n"); + bytes[12..16].copy_from_slice(b"IHDR"); + bytes[16..20].copy_from_slice(&width.to_be_bytes()); + bytes[20..24].copy_from_slice(&height.to_be_bytes()); + bytes + } + + #[test] + fn revision_is_stable_without_exposing_the_url() { + let url = "https://cdn.example/avatar.png?signature=secret"; + let revision = profile_avatar_revision(url).unwrap(); + assert_eq!(revision, profile_avatar_revision(url).unwrap()); + assert_eq!(revision.len(), 64); + assert!(!revision.contains("secret")); + } + + #[test] + fn url_and_ssrf_guards_reject_unsafe_targets() { + for url in [ + "http://cdn.example/avatar.png", + "https://user@cdn.example/avatar.png", + "https://cdn.example/avatar.png#fragment", + "https://cdn.example:0/avatar.png", + ] { + assert_eq!( + profile_avatar_revision(url), + Err(ProfileAvatarFetchError::UrlNotAllowed) + ); + } + for url in [ + "https://127.0.0.1/avatar.png", + "https://10.0.0.1/avatar.png", + "https://169.254.169.254/latest/meta-data", + "https://[::1]/avatar.png", + "https://[fe80::1]/avatar.png", + ] { + assert_eq!( + fetch_profile_avatar_blocking(url), + Err(ProfileAvatarFetchError::DialRejected), + "{url}" + ); + } + } + + #[test] + fn encoded_image_dimensions_are_bounded_before_proxying() { + assert_eq!(validate_profile_avatar_bytes(&png_header(16, 16)), Ok(())); + assert_eq!( + validate_profile_avatar_bytes(&png_header(MAX_AVATAR_SOURCE_EDGE_PX + 1, 1)), + Err(ProfileAvatarFetchError::TooLarge) + ); + assert_eq!( + validate_profile_avatar_bytes(b"not an image"), + Err(ProfileAvatarFetchError::InvalidImage) + ); + } + + #[test] + fn resolver_deadline_cancels_a_hanging_dial_future() { + let result = crate::chat_runtime::block_on_anywhere(with_timeout( + Duration::from_millis(1), + std::future::pending::<()>(), + )); + assert_eq!(result, Err(ProfileAvatarFetchError::TimedOut)); + } +} diff --git a/crates/op-host-services/src/provider_dial.rs b/crates/op-host-services/src/provider_dial.rs index eab3504c9..fe5576392 100644 --- a/crates/op-host-services/src/provider_dial.rs +++ b/crates/op-host-services/src/provider_dial.rs @@ -46,11 +46,25 @@ pub(crate) async fn client_for( ) -> Result { match policy { EndpointDialPolicy::Trusted => crate::chat_builtin_http::builtin_http_client(), - EndpointDialPolicy::PublicOnly => pinned_public_client(url).await, + EndpointDialPolicy::PublicOnly => pinned_public_client(url, false).await, } } -async fn pinned_public_client(url: &str) -> Result { +/// Build the stricter asset-only client used by bounded profile images. +/// +/// Clash-style fake-IP TUN resolvers use RFC 2544 `198.18.0.0/15` addresses as +/// opaque public-host tokens. This seam accepts only a hostname whose entire +/// resolution is in that range; literals and mixed sets remain rejected. +pub(crate) async fn client_for_tunnel_compatible_public_asset( + url: &str, +) -> Result { + pinned_public_client(url, true).await +} + +async fn pinned_public_client( + url: &str, + allow_tunnel_fake_ip: bool, +) -> Result { let parsed = reqwest::Url::parse(url).map_err(|_| ProviderDialError::NotAUrl)?; let host = parsed .host_str() @@ -61,6 +75,7 @@ async fn pinned_public_client(url: &str) -> Result().is_ok(); let addrs = if let Ok(ip) = host.parse::() { // Literal address: nothing to resolve, screening alone suffices. vec![SocketAddr::new(ip, port)] @@ -73,7 +88,8 @@ async fn pinned_public_client(url: &str) -> Result Result, +) -> Result, ProviderDialError> { + screen_resolved_addrs_for_policy(host, false, addrs, false) +} + +fn screen_resolved_addrs_for_policy( + host: &str, + host_is_literal: bool, + addrs: Vec, + allow_tunnel_fake_ip: bool, ) -> Result, ProviderDialError> { if addrs.is_empty() { return Err(ProviderDialError::Unresolved { host: host.to_string(), }); } + let tunnel_fake_ip_set = allow_tunnel_fake_ip + && !host_is_literal + && addrs + .iter() + .all(|address| is_rfc2544_benchmark_address(address.ip())); + if tunnel_fake_ip_set { + return Ok(addrs); + } if addrs .iter() .any(|addr| crate::web_credentials::is_restricted_ip(addr.ip())) @@ -111,6 +145,14 @@ pub(crate) fn screen_resolved_addrs( Ok(addrs) } +fn is_rfc2544_benchmark_address(address: std::net::IpAddr) -> bool { + matches!( + address, + std::net::IpAddr::V4(address) + if address.octets()[0] == 198 && matches!(address.octets()[1], 18 | 19) + ) +} + #[cfg(test)] mod tests { use super::*; @@ -184,6 +226,7 @@ mod tests { "192.168.1.1", "169.254.169.254", "168.63.129.16", + "198.18.0.42", "::1", "fd00:ec2::254", ] { @@ -194,4 +237,21 @@ mod tests { ); } } + + #[test] + fn bounded_asset_policy_accepts_only_hostname_fake_ip_sets() { + let fake_ip = vec![addr("198.18.0.42")]; + assert!( + screen_resolved_addrs_for_policy("avatar.example", false, fake_ip.clone(), true,) + .is_ok() + ); + assert!(screen_resolved_addrs_for_policy("198.18.0.42", true, fake_ip, true,).is_err()); + assert!(screen_resolved_addrs_for_policy( + "avatar.example", + false, + vec![addr("198.18.0.42"), addr("93.184.216.34")], + true, + ) + .is_err()); + } } diff --git a/crates/op-host-services/src/public_https_client.rs b/crates/op-host-services/src/public_https_client.rs index 995f827b2..a675c1435 100644 --- a/crates/op-host-services/src/public_https_client.rs +++ b/crates/op-host-services/src/public_https_client.rs @@ -1,9 +1,12 @@ //! Public-only HTTPS dialing for small, unauthenticated assets. //! //! Callers still own response limits and redirect policy. This module owns -//! the security-sensitive connect step: every hostname is resolved, every -//! address is screened, proxies are disabled, and the client is pinned to the -//! screened addresses so DNS rebinding cannot redirect the socket later. +//! the security-sensitive connect step: every hostname is resolved, addresses +//! are screened, proxies are disabled, and the client is pinned so DNS +//! rebinding cannot redirect the socket later. A separate current-account-only +//! seam recognizes an all-RFC-2544 hostname resolution as a Clash-style +//! fake-IP TUN token; literals, mixed sets, and every other reserved range +//! remain rejected. use std::fmt; @@ -49,6 +52,27 @@ pub async fn public_https_client( .map_err(|_| PublicHttpsClientError::DialRejected) } +/// Build the account-avatar-only variant that can traverse a Clash-style +/// fake-IP TUN. Callers must establish that the URL belongs to the local +/// authenticated account; remote collaboration data must use +/// [`public_https_client`]. +pub async fn tunnel_compatible_account_asset_client( + url: &reqwest::Url, +) -> Result { + if url.scheme() != "https" + || url.host_str().is_none() + || !url.username().is_empty() + || url.password().is_some() + || url.fragment().is_some() + || url.port() == Some(0) + { + return Err(PublicHttpsClientError::UrlNotAllowed); + } + crate::provider_dial::client_for_tunnel_compatible_public_asset(url.as_str()) + .await + .map_err(|_| PublicHttpsClientError::DialRejected) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/op-host-services/src/web_auth.rs b/crates/op-host-services/src/web_auth.rs index ca15ac673..7d7459ea4 100644 --- a/crates/op-host-services/src/web_auth.rs +++ b/crates/op-host-services/src/web_auth.rs @@ -73,8 +73,12 @@ pub(crate) fn status(state: &mut WebCanvasState) -> WebReply { AuthStatus::SignedIn { display_name, primary_email, + avatar_url, .. } => { + let avatar_revision = avatar_url + .as_deref() + .and_then(|url| crate::profile_avatar_fetch::profile_avatar_revision(url).ok()); state.editor.editor_ui.account = AccountState::SignedIn { handle: primary_email .clone() @@ -86,6 +90,7 @@ pub(crate) fn status(state: &mut WebCanvasState) -> WebReply { "signed_in": true, "display_name": display_name, "primary_email": primary_email, + "avatar_revision": avatar_revision, }) } _ => { @@ -253,8 +258,12 @@ pub(crate) fn login_status(state: &mut WebCanvasState) -> WebReply { AuthStatus::SignedIn { display_name, primary_email, + avatar_url, .. } => { + let avatar_revision = avatar_url + .as_deref() + .and_then(|url| crate::profile_avatar_fetch::profile_avatar_revision(url).ok()); state.auth_login_handle = None; state.editor.editor_ui.account = AccountState::SignedIn { handle: primary_email @@ -266,6 +275,7 @@ pub(crate) fn login_status(state: &mut WebCanvasState) -> WebReply { "state": "signed_in", "display_name": display_name, "primary_email": primary_email, + "avatar_revision": avatar_revision, }) } AuthStatus::Error { code } => { @@ -292,6 +302,49 @@ pub(crate) fn login_cancel(state: &mut WebCanvasState) -> WebReply { ok() } +/// `POST /api/auth/avatar` — proxy the current profile image through the +/// daemon's public-only HTTPS client. The browser receives only an opaque +/// revision and bounded bytes, never the signed CDN URL. +pub(crate) fn avatar() -> WebReply { + if !op_auth_bridge::available() { + return WebReply { + status: "404 Not Found", + body: crate::mcp_serve::rest_error_body("account avatar unavailable"), + }; + } + let AuthStatus::SignedIn { + avatar_url: Some(url), + .. + } = op_auth_bridge::poll(op_auth_bridge::SESSION_HANDLE) + else { + return WebReply { + status: "404 Not Found", + body: crate::mcp_serve::rest_error_body("account avatar unavailable"), + }; + }; + let Ok(revision) = crate::profile_avatar_fetch::profile_avatar_revision(&url) else { + return WebReply { + status: "404 Not Found", + body: crate::mcp_serve::rest_error_body("account avatar unavailable"), + }; + }; + let Ok(bytes) = crate::profile_avatar_fetch::fetch_account_avatar_blocking(&url) else { + return WebReply { + status: "502 Bad Gateway", + body: crate::mcp_serve::rest_error_body("account avatar fetch failed"), + }; + }; + use base64::Engine as _; + WebReply { + status: "200 OK", + body: serde_json::json!({ + "revision": revision, + "encoded": base64::engine::general_purpose::STANDARD.encode(bytes), + }) + .to_string(), + } +} + /// `POST /api/auth/logout` — drop the shared session (revokes the device /// token server-side on a background thread inside the library). pub(crate) fn logout(state: &mut WebCanvasState) -> WebReply { diff --git a/crates/op-host-services/src/web_canvas_server/connection.rs b/crates/op-host-services/src/web_canvas_server/connection.rs index c5270e61e..5f1764d78 100644 --- a/crates/op-host-services/src/web_canvas_server/connection.rs +++ b/crates/op-host-services/src/web_canvas_server/connection.rs @@ -112,6 +112,18 @@ pub(super) fn serve_one( )?; return Ok(false); } + // Current-account avatar proxy: performs bounded public HTTPS I/O on this + // connection thread, never while holding the editor-state mutex. + if req.method == "POST" && req.path == op_editor_core::auth_routes::AVATAR { + let reply = crate::web_auth::avatar(); + crate::mcp_serve::write_mcp_http_response_with_origin( + stream, + reply.status, + &reply.body, + cors_origin, + )?; + return Ok(false); + } // Device-login begin: waits (per-connection thread, off the state // lock) for the pairing's verification URI so the popup can navigate // straight from this response — handled here rather than in the diff --git a/crates/op-host-services/src/web_canvas_server_tests.rs b/crates/op-host-services/src/web_canvas_server_tests.rs index 7bbe8494c..6b3e18d3a 100644 --- a/crates/op-host-services/src/web_canvas_server_tests.rs +++ b/crates/op-host-services/src/web_canvas_server_tests.rs @@ -325,6 +325,40 @@ fn cross_origin_browser_cannot_write_server_credentials() { assert_eq!(before, crate::settings_io::fingerprint(&guard.editor)); } +#[test] +fn cross_origin_browser_cannot_read_account_avatar() { + let state = Mutex::new(fresh_state()); + let request = format!( + "POST {} HTTP/1.1\r\nHost: 127.0.0.1:3100\r\nOrigin: https://evil.example\r\nContent-Type: application/json\r\nContent-Length: 2\r\n\r\n{{}}", + op_editor_core::auth_routes::AVATAR + ); + let request_len = request.len(); + let mut stream = std::io::Cursor::new(request.into_bytes()); + + serve_one(&mut stream, &state, &SseHub::default()).expect("request handled"); + + let response = String::from_utf8_lossy(&stream.get_ref()[request_len..]); + assert!(response.contains("403 Forbidden")); + assert!(response.contains("cross-origin")); +} + +#[test] +fn account_avatar_proxy_rejects_subresource_gets_without_fetching() { + let state = Mutex::new(fresh_state()); + let request = format!( + "GET {} HTTP/1.1\r\nHost: 127.0.0.1:3100\r\nContent-Length: 0\r\n\r\n", + op_editor_core::auth_routes::AVATAR + ); + let request_len = request.len(); + let mut stream = std::io::Cursor::new(request.into_bytes()); + + serve_one(&mut stream, &state, &SseHub::default()).expect("request handled"); + + let response = String::from_utf8_lossy(&stream.get_ref()[request_len..]); + assert!(response.contains("404 Not Found")); + assert!(!response.contains("\"encoded\"")); +} + #[test] fn credential_origin_check_allows_default_loopback_and_non_browser_clients() { for headers in [ diff --git a/crates/op-host-web/src/image_decode_queue.rs b/crates/op-host-web/src/image_decode_queue.rs index 1eaded8dd..21c9a22ce 100644 --- a/crates/op-host-web/src/image_decode_queue.rs +++ b/crates/op-host-web/src/image_decode_queue.rs @@ -16,15 +16,19 @@ pub(crate) struct WebDecodeJob { pub(crate) fn take_web_decode_batch(max: usize) -> Vec { take_pending_decodes(max) .into_iter() - .filter_map(|pending| match cached_bytes_for(pending.id) { - Some(bytes) => Some(WebDecodeJob { - id: pending.id, - bytes, - max_edge_px: pending.max_edge_px, - }), - None => { - mark_decode_done(pending.id); - None + .filter_map(|pending| { + match cached_bytes_for(pending.id).or_else(|| { + op_editor_ui::collab_avatar_runtime::cached_collab_avatar_bytes(pending.id) + }) { + Some(bytes) => Some(WebDecodeJob { + id: pending.id, + bytes, + max_edge_px: pending.max_edge_px, + }), + None => { + mark_decode_done(pending.id); + None + } } }) .collect() @@ -101,4 +105,34 @@ mod tests { "a failed bridge decode must remain negatively cached" ); } + + #[test] + fn web_decode_batch_accepts_bounded_profile_avatar_bytes() { + let _decode_guard = DECODE_REGISTRY_LOCK + .lock() + .unwrap_or_else(|error| error.into_inner()); + let _ = op_editor_ui::collab_avatar_runtime::begin_collab_avatar_generation(0xACCA_0001); + let _ = op_editor_ui::collab_avatar_runtime::register_account_avatar_url(None); + clear_pending_work(); + let mut png = vec![0; 32]; + png[..8].copy_from_slice(b"\x89PNG\r\n\x1a\n"); + png[12..16].copy_from_slice(b"IHDR"); + png[16..20].copy_from_slice(&16_u32.to_be_bytes()); + png[20..24].copy_from_slice(&16_u32.to_be_bytes()); + assert!( + op_editor_ui::collab_avatar_runtime::install_account_avatar_bytes( + "web-avatar-revision", + png + ) + ); + let image = op_editor_ui::collab_avatar_runtime::account_avatar_image() + .expect("ready account avatar"); + note_pending_decode(image.image_id, 64); + + let batch = take_web_decode_batch(1); + assert_eq!(batch.len(), 1); + assert_eq!(batch[0].id, image.image_id); + finish_web_decode(batch[0].id, true); + clear_pending_work(); + } } diff --git a/crates/op-host-web/src/web_auth_sync.rs b/crates/op-host-web/src/web_auth_sync.rs index 780831f0d..79b2b0d84 100644 --- a/crates/op-host-web/src/web_auth_sync.rs +++ b/crates/op-host-web/src/web_auth_sync.rs @@ -51,6 +51,13 @@ thread_local! { /// The begin POST is in flight — status polls are suppressed so they /// can't observe the daemon's pre-begin `idle` state. static BEGIN_INFLIGHT: Cell = const { Cell::new(false) }; + /// Opaque avatar revision requested by the latest authenticated status. + static ACCOUNT_AVATAR_DESIRED: RefCell> = const { RefCell::new(None) }; + /// Revision currently installed in the bounded profile-avatar cache. + static ACCOUNT_AVATAR_INSTALLED: RefCell> = const { RefCell::new(None) }; + /// At most one request per revision; a changed revision may supersede an + /// older in-flight request, whose callback is discarded. + static ACCOUNT_AVATAR_IN_FLIGHT: RefCell> = const { RefCell::new(None) }; } /// Shared latches for the interval tick. @@ -168,6 +175,7 @@ fn fetch_status(inner: &Rc>, base: &str) let Ok(parsed) = serde_json::from_str::(&body) else { return; }; + sync_account_avatar(&inner, parsed["avatar_revision"].as_str()); let mut b = inner.borrow_mut(); let ui = &mut b.host_mut().editor_state_mut().editor_ui; let available = parsed["available"].as_bool().unwrap_or(false); @@ -211,7 +219,10 @@ fn drain_actions(inner: &Rc>, base: &str close_login_popup_placeholder(); auth_routes::LOGIN_CANCEL } - PendingAuthAction::SignOut => auth_routes::LOGOUT, + PendingAuthAction::SignOut => { + clear_account_avatar_sync_state(); + auth_routes::LOGOUT + } }; let _ = live_sync::post_json(&format!("{base}{path}"), "{}", None); } @@ -281,6 +292,7 @@ fn apply_login_status( } "exchanging" => ui.login_modal_status = Some(LoginFlowStatus::Exchanging), "signed_in" => { + sync_account_avatar(inner, parsed["avatar_revision"].as_str()); let display_name = parsed["display_name"] .as_str() .unwrap_or_default() @@ -329,3 +341,125 @@ fn apply_login_status( let _ = b.repaint(); } } + +fn sync_account_avatar( + inner: &Rc>, + revision: Option<&str>, +) { + let revision = revision + .filter(|value| { + !value.is_empty() + && value.len() <= 128 + && value + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_')) + }) + .map(str::to_owned); + ACCOUNT_AVATAR_DESIRED.with(|desired| *desired.borrow_mut() = revision.clone()); + let Some(revision) = revision else { + clear_account_avatar_sync_state(); + return; + }; + if account_avatar_revision_active(&revision) { + return; + } + + ACCOUNT_AVATAR_INSTALLED.with(|installed| installed.borrow_mut().take()); + let _ = op_editor_ui::collab_avatar_runtime::register_account_avatar_url(None); + ACCOUNT_AVATAR_IN_FLIGHT.with(|in_flight| { + *in_flight.borrow_mut() = Some(revision.clone()); + }); + let expected = revision.clone(); + let inner = inner.clone(); + let url = format!( + "{}{}", + crate::daemon_base::daemon_base(), + auth_routes::AVATAR + ); + let started = live_sync::post_json_with_status( + &url, + "{}", + Rc::new(move |status, body| { + ACCOUNT_AVATAR_IN_FLIGHT.with(|in_flight| { + if in_flight.borrow().as_deref() == Some(expected.as_str()) { + in_flight.borrow_mut().take(); + } + }); + let still_desired = ACCOUNT_AVATAR_DESIRED + .with(|desired| desired.borrow().as_deref() == Some(expected.as_str())); + if status != 200 || !still_desired { + return; + } + let Ok(parsed) = serde_json::from_str::(&body) else { + return; + }; + if parsed["revision"].as_str() != Some(expected.as_str()) { + return; + } + use base64::Engine as _; + let Some(encoded) = parsed["encoded"].as_str().and_then(|value| { + base64::engine::general_purpose::STANDARD + .decode(value.as_bytes()) + .ok() + }) else { + return; + }; + if !op_editor_ui::collab_avatar_runtime::install_account_avatar_bytes( + &expected, encoded, + ) { + return; + } + ACCOUNT_AVATAR_INSTALLED.with(|installed| { + *installed.borrow_mut() = Some(expected.clone()); + }); + let mut context = inner.borrow_mut(); + context.host_mut().mark_editor_state_dirty(); + let _ = context.repaint(); + }), + ); + if !started { + ACCOUNT_AVATAR_IN_FLIGHT.with(|in_flight| in_flight.borrow_mut().take()); + } +} + +fn clear_account_avatar_sync_state() { + clear_account_avatar_revision_latches(); + let _ = op_editor_ui::collab_avatar_runtime::register_account_avatar_url(None); +} + +fn clear_account_avatar_revision_latches() { + ACCOUNT_AVATAR_DESIRED.with(|desired| desired.borrow_mut().take()); + ACCOUNT_AVATAR_INSTALLED.with(|installed| installed.borrow_mut().take()); + ACCOUNT_AVATAR_IN_FLIGHT.with(|in_flight| in_flight.borrow_mut().take()); +} + +fn account_avatar_revision_active(revision: &str) -> bool { + ACCOUNT_AVATAR_INSTALLED.with(|installed| installed.borrow().as_deref() == Some(revision)) + || ACCOUNT_AVATAR_IN_FLIGHT + .with(|in_flight| in_flight.borrow().as_deref() == Some(revision)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sign_out_latches_allow_same_revision_to_be_fetched_after_relogin() { + const REVISION: &str = "same-account-revision"; + ACCOUNT_AVATAR_DESIRED.with(|desired| { + *desired.borrow_mut() = Some(REVISION.to_string()); + }); + ACCOUNT_AVATAR_INSTALLED.with(|installed| { + *installed.borrow_mut() = Some(REVISION.to_string()); + }); + ACCOUNT_AVATAR_IN_FLIGHT.with(|in_flight| { + *in_flight.borrow_mut() = Some(REVISION.to_string()); + }); + assert!(account_avatar_revision_active(REVISION)); + + clear_account_avatar_revision_latches(); + + assert!(!account_avatar_revision_active(REVISION)); + ACCOUNT_AVATAR_DESIRED.with(|desired| assert!(desired.borrow().is_none())); + } +}