diff --git a/crates/ely_app/src/shell/internal_pages/downloads.rs b/crates/ely_app/src/shell/internal_pages/downloads.rs index 4f6fe4a..28242d0 100644 --- a/crates/ely_app/src/shell/internal_pages/downloads.rs +++ b/crates/ely_app/src/shell/internal_pages/downloads.rs @@ -1,6 +1,9 @@ use ely_browser_core::BrowserSnapshot; use ely_design_system::colors; -use ely_domain::{DownloadEntry, DownloadState}; +use ely_domain::{ + DownloadDestination, DownloadEntry, DownloadPolicy, DownloadSecurity, DownloadState, +}; +use gpui::prelude::FluentBuilder; use gpui::{ AnyElement, InteractiveElement, IntoElement, ParentElement, SharedString, Styled, div, px, rgb, }; @@ -42,9 +45,25 @@ impl ElyShell { ) .child( div() - .text_xs() - .text_color(rgb(colors::MUTED)) - .child(format!("{} downloads", snapshot.download_entries.len())), + .flex() + .flex_col() + .items_end() + .gap_1() + .child(div().text_xs().text_color(rgb(colors::MUTED)).child( + format!("{} downloads", snapshot.download_entries.len()), + )) + .child( + div() + .flex() + .items_center() + .gap_1() + .text_xs() + .text_color(rgb(colors::MUTED)) + .child(IconName::Folder) + .child(div().max_w(px(360.0)).truncate().child( + download_policy_label(&snapshot.active_download_policy), + )), + ), ), ) .child(render_downloads_list(snapshot)), @@ -120,6 +139,23 @@ fn render_download_row(index: usize, entry: &DownloadEntry) -> AnyElement { .truncate() .text_color(rgb(colors::MUTED)) .child(entry.source_url().display_url()), + ) + .child( + div() + .flex() + .items_center() + .gap_2() + .text_xs() + .text_color(rgb(colors::MUTED)) + .child( + div() + .min_w_0() + .truncate() + .child(download_destination_label(entry.destination())), + ) + .when(entry.security().requires_prompt(), |this| { + this.child(render_security_prompt(entry.security())) + }), ), ), ) @@ -165,6 +201,35 @@ fn download_size_label(entry: &DownloadEntry) -> String { } } +fn download_policy_label(policy: &DownloadPolicy) -> String { + format!("Profile path: {}", download_destination_label(policy.destination())) +} + +fn download_destination_label(destination: &DownloadDestination) -> String { + match destination { + DownloadDestination::AskEveryTime => "Ask before saving".to_string(), + DownloadDestination::FixedDirectory(path) => path.display().to_string(), + } +} + +fn render_security_prompt(security: &DownloadSecurity) -> AnyElement { + div() + .flex() + .items_center() + .gap_1() + .text_color(rgb(colors::ERROR)) + .child(IconName::TriangleAlert) + .child(download_security_label(security)) + .into_any_element() +} + +fn download_security_label(security: &DownloadSecurity) -> &'static str { + match security { + DownloadSecurity::Standard => "Standard", + DownloadSecurity::DangerousExtension => "Extension prompt required", + } +} + fn format_bytes(bytes: u64) -> String { const KIB: u64 = 1024; const MIB: u64 = KIB * 1024; diff --git a/crates/ely_browser_core/src/state.rs b/crates/ely_browser_core/src/state.rs index fa1ff32..ee9e755 100644 --- a/crates/ely_browser_core/src/state.rs +++ b/crates/ely_browser_core/src/state.rs @@ -1,8 +1,8 @@ use std::collections::BTreeMap; use ely_domain::{ - ArchivedTab, BrowserTab, DomainError, DownloadEntry, HistoryEntry, Profile, ProfileId, - ProfileKind, Space, SpaceId, TabId, UrlText, + ArchivedTab, BrowserTab, DomainError, DownloadEntry, DownloadPolicy, HistoryEntry, Profile, + ProfileId, ProfileKind, Space, SpaceId, TabId, UrlText, }; use crate::CoreError; @@ -45,6 +45,7 @@ pub struct BrowserSnapshot { pub active_space_id: SpaceId, pub active_space_name: String, pub active_profile_name: String, + pub active_download_policy: DownloadPolicy, pub command_query: String, } @@ -191,10 +192,18 @@ impl BrowserCore { active_space_id: self.active_space_id.clone(), active_space_name: active_space.name().to_string(), active_profile_name: active_profile.name().to_string(), + active_download_policy: active_profile.download_policy().clone(), command_query: self.command_query.clone(), }) } + fn active_profile(&self) -> Result<&Profile, CoreError> { + self.profiles + .iter() + .find(|profile| profile.id() == &self.active_profile_id) + .ok_or(CoreError::MissingActiveTab) + } + fn favorites(&self) -> Vec { self.tabs.iter().filter(|tab| tab.flags().favorite).cloned().collect() } diff --git a/crates/ely_browser_core/src/state/downloads.rs b/crates/ely_browser_core/src/state/downloads.rs index c2d9f3d..2ff5898 100644 --- a/crates/ely_browser_core/src/state/downloads.rs +++ b/crates/ely_browser_core/src/state/downloads.rs @@ -13,10 +13,12 @@ impl BrowserCore { file_name: impl Into, total_bytes: Option, ) -> Result { + let destination = self.active_profile()?.download_policy().destination().clone(); let entry = DownloadEntry::started( self.active_profile_id.clone(), source_url, file_name, + destination, total_bytes, SystemTime::now(), )?; diff --git a/crates/ely_browser_core/src/state/profiles.rs b/crates/ely_browser_core/src/state/profiles.rs index a2078ef..4d017bc 100644 --- a/crates/ely_browser_core/src/state/profiles.rs +++ b/crates/ely_browser_core/src/state/profiles.rs @@ -1,4 +1,4 @@ -use ely_domain::{Profile, ProfileId, ProfileKind, TabId}; +use ely_domain::{DownloadPolicy, Profile, ProfileId, ProfileKind, TabId}; use crate::CoreError; @@ -66,4 +66,19 @@ impl BrowserCore { self.select_tab(&tab_id)?; Ok(tab_id) } + + pub fn set_profile_download_policy( + &mut self, + profile_id: &ProfileId, + download_policy: DownloadPolicy, + ) -> Result<(), CoreError> { + let profile = self + .profiles + .iter_mut() + .find(|profile| profile.id() == profile_id) + .ok_or_else(|| CoreError::ProfileNotFound { id: profile_id.clone() })?; + + profile.set_download_policy(download_policy); + Ok(()) + } } diff --git a/crates/ely_browser_core/tests/downloads.rs b/crates/ely_browser_core/tests/downloads.rs index a2d79f6..9af9599 100644 --- a/crates/ely_browser_core/tests/downloads.rs +++ b/crates/ely_browser_core/tests/downloads.rs @@ -1,7 +1,10 @@ use std::error::Error; use ely_browser_core::{BrowserCore, CoreError, InitialBrowserConfig}; -use ely_domain::{DomainError, DownloadId, DownloadState, ProfileKind, UrlText}; +use ely_domain::{ + DomainError, DownloadDestination, DownloadId, DownloadPolicy, DownloadSecurity, DownloadState, + ProfileKind, UrlText, +}; #[test] fn download_entries_stay_with_active_profile() -> Result<(), Box> { @@ -62,6 +65,79 @@ fn controls_download_lifecycle() -> Result<(), Box> { Ok(()) } +#[test] +fn records_active_profile_download_policy_on_started_entry() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.active_tab()?.profile_id().clone(); + let policy = DownloadPolicy::fixed_directory("/tmp/ely-work-downloads")?; + core.set_profile_download_policy(&profile_id, policy.clone())?; + + core.record_download_started( + UrlText::parse("https://example.com/installer.dmg")?, + "installer.dmg", + Some(4096), + )?; + + let snapshot = core.snapshot()?; + let entry = active_download(&core)?; + assert_eq!(snapshot.active_download_policy, policy); + assert_eq!(entry.destination(), policy.destination()); + assert_eq!(entry.security(), &DownloadSecurity::DangerousExtension); + Ok(()) +} + +#[test] +fn download_policies_stay_with_profile() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let default_profile_id = core.active_tab()?.profile_id().clone(); + let default_policy = DownloadPolicy::fixed_directory("/tmp/ely-default-downloads")?; + core.set_profile_download_policy(&default_profile_id, default_policy.clone())?; + + let personal_profile_id = core.create_profile("Personal", 0xf54e00, ProfileKind::Standard)?; + assert_eq!( + core.snapshot()?.active_download_policy.destination(), + &DownloadDestination::AskEveryTime + ); + + let personal_policy = DownloadPolicy::fixed_directory("/tmp/ely-personal-downloads")?; + core.set_profile_download_policy(&personal_profile_id, personal_policy.clone())?; + assert_eq!(core.snapshot()?.active_download_policy, personal_policy); + + core.select_profile(&default_profile_id)?; + assert_eq!(core.snapshot()?.active_download_policy, default_policy); + Ok(()) +} + +#[test] +fn rejects_relative_download_directory() -> Result<(), Box> { + let error = match DownloadPolicy::fixed_directory("downloads") { + Ok(_) => return Err("relative download directory should be rejected".into()), + Err(error) => error, + }; + + assert_eq!(error, DomainError::InvalidDownloadDirectory { path: "downloads".to_string() }); + Ok(()) +} + +#[test] +fn rejects_path_like_download_file_name() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let error = match core.record_download_started( + UrlText::parse("https://example.com/evil.sh")?, + "../evil.sh", + None, + ) { + Ok(_) => return Err("path-like download file name should be rejected".into()), + Err(error) => error, + }; + + assert_eq!( + error, + CoreError::Domain(DomainError::InvalidFileName { value: "../evil.sh".to_string() }) + ); + Ok(()) +} + #[test] fn retries_cancelled_download_from_zero_bytes() -> Result<(), Box> { let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; diff --git a/crates/ely_domain/src/download.rs b/crates/ely_domain/src/download.rs index 7e0b9b5..2f5f895 100644 --- a/crates/ely_domain/src/download.rs +++ b/crates/ely_domain/src/download.rs @@ -1,4 +1,7 @@ -use std::time::SystemTime; +use std::{ + path::{Path, PathBuf}, + time::SystemTime, +}; use crate::{DomainError, DownloadId, ProfileId, UrlText}; @@ -11,23 +14,126 @@ pub enum DownloadState { Failed, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum DownloadDestination { + AskEveryTime, + FixedDirectory(PathBuf), +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct DownloadPolicy { + destination: DownloadDestination, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum DownloadSecurity { + Standard, + DangerousExtension, +} + #[derive(Clone, Debug, Eq, PartialEq)] pub struct DownloadEntry { id: DownloadId, profile_id: ProfileId, source_url: UrlText, file_name: String, + destination: DownloadDestination, + security: DownloadSecurity, state: DownloadState, received_bytes: u64, total_bytes: Option, started_at: SystemTime, } +const DANGEROUS_DOWNLOAD_EXTENSIONS: &[&str] = &[ + "app", + "applescript", + "bat", + "bash", + "cmd", + "com", + "command", + "dmg", + "exe", + "fish", + "jar", + "js", + "jse", + "msi", + "msp", + "pkg", + "ps1", + "psm1", + "reg", + "scr", + "scpt", + "sh", + "terminal", + "vbe", + "vbs", + "workflow", + "zsh", +]; + +impl DownloadDestination { + pub fn fixed_directory(path: impl Into) -> Result { + let path = path.into(); + if path.as_os_str().is_empty() { + return Err(DomainError::EmptyField { field: "download_directory" }); + } + if !path.is_absolute() { + return Err(DomainError::InvalidDownloadDirectory { path: path.display().to_string() }); + } + + Ok(Self::FixedDirectory(path)) + } + + #[must_use] + pub fn as_path(&self) -> Option<&Path> { + match self { + Self::AskEveryTime => None, + Self::FixedDirectory(path) => Some(path.as_path()), + } + } +} + +impl DownloadPolicy { + #[must_use] + pub fn ask_every_time() -> Self { + Self { destination: DownloadDestination::AskEveryTime } + } + + pub fn fixed_directory(path: impl Into) -> Result { + Ok(Self { destination: DownloadDestination::fixed_directory(path)? }) + } + + #[must_use] + pub fn destination(&self) -> &DownloadDestination { + &self.destination + } +} + +impl DownloadSecurity { + #[must_use] + pub fn for_file_name(file_name: &str) -> Self { + match download_extension(file_name) { + Some(extension) if is_dangerous_extension(extension) => Self::DangerousExtension, + _ => Self::Standard, + } + } + + #[must_use] + pub fn requires_prompt(&self) -> bool { + matches!(self, Self::DangerousExtension) + } +} + impl DownloadEntry { pub fn started( profile_id: ProfileId, source_url: UrlText, file_name: impl Into, + destination: DownloadDestination, total_bytes: Option, started_at: SystemTime, ) -> Result { @@ -36,12 +142,15 @@ impl DownloadEntry { if file_name.is_empty() { return Err(DomainError::EmptyField { field: "file_name" }); } + validate_file_name(file_name)?; Ok(Self { id: DownloadId::new(), profile_id, source_url, file_name: file_name.to_string(), + destination, + security: DownloadSecurity::for_file_name(file_name), state: DownloadState::InProgress, received_bytes: 0, total_bytes, @@ -115,6 +224,16 @@ impl DownloadEntry { &self.file_name } + #[must_use] + pub fn destination(&self) -> &DownloadDestination { + &self.destination + } + + #[must_use] + pub fn security(&self) -> &DownloadSecurity { + &self.security + } + #[must_use] pub fn state(&self) -> &DownloadState { &self.state @@ -170,3 +289,28 @@ impl DownloadState { } } } + +fn validate_file_name(file_name: &str) -> Result<(), DomainError> { + let has_path_separator = file_name.chars().any(|ch| ch == '/' || ch == '\\'); + let is_parent_reference = file_name == "." || file_name == ".."; + let has_control_character = file_name.chars().any(char::is_control); + + if has_path_separator || is_parent_reference || has_control_character { + return Err(DomainError::InvalidFileName { value: file_name.to_string() }); + } + + Ok(()) +} + +fn download_extension(file_name: &str) -> Option<&str> { + let (_, extension) = file_name.rsplit_once('.')?; + if extension.is_empty() { + return None; + } + + Some(extension) +} + +fn is_dangerous_extension(extension: &str) -> bool { + DANGEROUS_DOWNLOAD_EXTENSIONS.iter().any(|candidate| extension.eq_ignore_ascii_case(candidate)) +} diff --git a/crates/ely_domain/src/error.rs b/crates/ely_domain/src/error.rs index 1ad3f12..cbea4fb 100644 --- a/crates/ely_domain/src/error.rs +++ b/crates/ely_domain/src/error.rs @@ -8,6 +8,12 @@ pub enum DomainError { #[error("invalid URL: {value}")] InvalidUrl { value: String }, + #[error("invalid file name: {value}")] + InvalidFileName { value: String }, + + #[error("invalid download directory: {path}")] + InvalidDownloadDirectory { path: String }, + #[error("invalid command query")] InvalidCommand, diff --git a/crates/ely_domain/src/lib.rs b/crates/ely_domain/src/lib.rs index b20f636..1d68940 100644 --- a/crates/ely_domain/src/lib.rs +++ b/crates/ely_domain/src/lib.rs @@ -12,7 +12,9 @@ mod url_text; pub use archive::{ArchiveSource, ArchivedTab}; pub use command::{CommandIntent, CommandScope}; -pub use download::{DownloadEntry, DownloadState}; +pub use download::{ + DownloadDestination, DownloadEntry, DownloadPolicy, DownloadSecurity, DownloadState, +}; pub use error::DomainError; pub use history::HistoryEntry; pub use identifiers::{DownloadId, ProfileId, SpaceId, SplitId, TabId, WebViewId}; diff --git a/crates/ely_domain/src/profile.rs b/crates/ely_domain/src/profile.rs index b236f6c..20385a5 100644 --- a/crates/ely_domain/src/profile.rs +++ b/crates/ely_domain/src/profile.rs @@ -1,4 +1,4 @@ -use crate::ProfileId; +use crate::{DownloadPolicy, ProfileId}; #[derive(Clone, Debug, Eq, PartialEq)] pub enum ProfileKind { @@ -12,12 +12,19 @@ pub struct Profile { name: String, color_hex: u32, kind: ProfileKind, + download_policy: DownloadPolicy, } impl Profile { #[must_use] pub fn new(name: impl Into, color_hex: u32, kind: ProfileKind) -> Self { - Self { id: ProfileId::new(), name: name.into(), color_hex, kind } + Self { + id: ProfileId::new(), + name: name.into(), + color_hex, + kind, + download_policy: DownloadPolicy::ask_every_time(), + } } #[must_use] @@ -39,4 +46,13 @@ impl Profile { pub fn kind(&self) -> &ProfileKind { &self.kind } + + #[must_use] + pub fn download_policy(&self) -> &DownloadPolicy { + &self.download_policy + } + + pub fn set_download_policy(&mut self, download_policy: DownloadPolicy) { + self.download_policy = download_policy; + } }