From d33c1e6a302aa30d4d137fead8eb52c8005ef0dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 7 May 2026 22:27:20 -0400 Subject: [PATCH] Confirm dangerous downloads --- crates/ely_app/src/services/download_files.rs | 2 +- crates/ely_app/src/shell/downloads.rs | 216 ++++++++++++++++++ .../shell/internal_pages/download_labels.rs | 12 +- .../src/shell/internal_pages/downloads.rs | 81 ++++++- crates/ely_app/src/shell/mod.rs | 106 +-------- .../ely_browser_core/src/state/downloads.rs | 15 ++ crates/ely_browser_core/tests/downloads.rs | 57 +++++ crates/ely_domain/src/download.rs | 18 ++ 8 files changed, 393 insertions(+), 114 deletions(-) create mode 100644 crates/ely_app/src/shell/downloads.rs diff --git a/crates/ely_app/src/services/download_files.rs b/crates/ely_app/src/services/download_files.rs index 9e8630e..5f2d93f 100644 --- a/crates/ely_app/src/services/download_files.rs +++ b/crates/ely_app/src/services/download_files.rs @@ -58,7 +58,7 @@ impl DownloadFileAction { command } - fn label(self) -> &'static str { + pub fn label(self) -> &'static str { match self { Self::Open => "open", Self::Reveal => "reveal", diff --git a/crates/ely_app/src/shell/downloads.rs b/crates/ely_app/src/shell/downloads.rs new file mode 100644 index 0000000..95764ba --- /dev/null +++ b/crates/ely_app/src/shell/downloads.rs @@ -0,0 +1,216 @@ +use ely_domain::DownloadId; +use gpui::Context; + +use crate::services::{ + download_checksums::DownloadChecksumCalculator, download_files::DownloadFileAction, +}; + +use super::{ElyShell, ShellState}; + +#[derive(Clone, Debug)] +pub(super) struct PendingDownloadFileAction { + download_id: DownloadId, + file_name: String, + action: DownloadFileAction, +} + +impl PendingDownloadFileAction { + fn new(download_id: DownloadId, file_name: String, action: DownloadFileAction) -> Self { + Self { download_id, file_name, action } + } + + pub(super) fn file_name(&self) -> &str { + &self.file_name + } + + pub(super) fn action_label(&self) -> &'static str { + self.action.label() + } +} + +impl ElyShell { + pub(super) fn pause_download(&mut self, download_id: &DownloadId, cx: &mut Context) { + if let ShellState::Ready(core) = &mut self.state + && core.pause_download(download_id).is_ok() + { + cx.notify(); + } + } + + pub(super) fn resume_download(&mut self, download_id: &DownloadId, cx: &mut Context) { + if let ShellState::Ready(core) = &mut self.state + && core.resume_download(download_id).is_ok() + { + cx.notify(); + } + } + + pub(super) fn cancel_download(&mut self, download_id: &DownloadId, cx: &mut Context) { + if let ShellState::Ready(core) = &mut self.state + && core.cancel_download(download_id).is_ok() + { + cx.notify(); + } + } + + pub(super) fn retry_download(&mut self, download_id: &DownloadId, cx: &mut Context) { + if let ShellState::Ready(core) = &mut self.state + && core.retry_download(download_id).is_ok() + { + cx.notify(); + } + } + + pub(super) fn request_clear_active_profile_downloads(&mut self, cx: &mut Context) { + self.download_clear_confirmation = true; + cx.notify(); + } + + pub(super) fn cancel_clear_active_profile_downloads(&mut self, cx: &mut Context) { + self.download_clear_confirmation = false; + cx.notify(); + } + + pub(super) fn clear_active_profile_downloads(&mut self, cx: &mut Context) { + if let ShellState::Ready(core) = &mut self.state { + core.clear_downloads_for_active_profile(); + } + self.download_clear_confirmation = false; + self.download_action_error = None; + cx.notify(); + } + + pub(super) fn calculate_download_checksum( + &mut self, + download_id: &DownloadId, + cx: &mut Context, + ) { + let result = match &mut self.state { + ShellState::Ready(core) => core + .download_target_file_path(download_id) + .map_err(|error| error.to_string()) + .and_then(|path| { + DownloadChecksumCalculator::sha256(&path).map_err(|error| error.to_string()) + }) + .and_then(|checksum| { + core.record_download_checksum(download_id, checksum) + .map_err(|error| error.to_string()) + }), + ShellState::StartupError(message) => Err(message.clone()), + }; + + self.download_action_error = result.err(); + cx.notify(); + } + + pub(super) fn open_download_file(&mut self, download_id: &DownloadId, cx: &mut Context) { + self.run_download_file_action(download_id, DownloadFileAction::Open, cx); + } + + pub(super) fn reveal_download_file( + &mut self, + download_id: &DownloadId, + cx: &mut Context, + ) { + self.run_download_file_action(download_id, DownloadFileAction::Reveal, cx); + } + + pub(super) fn confirm_download_security_prompt(&mut self, cx: &mut Context) { + let Some(pending_action) = self.download_security_confirmation.take() else { + cx.notify(); + return; + }; + + let result = match &mut self.state { + ShellState::Ready(core) => core + .confirm_download_security_prompt(&pending_action.download_id) + .map_err(|error| error.to_string()), + ShellState::StartupError(message) => Err(message.clone()), + }; + + if let Err(message) = result { + self.download_action_error = Some(message); + cx.notify(); + return; + } + + self.execute_download_file_action(&pending_action.download_id, pending_action.action, cx); + } + + pub(super) fn cancel_download_security_prompt(&mut self, cx: &mut Context) { + self.download_security_confirmation = None; + cx.notify(); + } + + fn run_download_file_action( + &mut self, + download_id: &DownloadId, + action: DownloadFileAction, + cx: &mut Context, + ) { + match self.queue_security_confirmation(download_id, action) { + Ok(true) => { + self.download_action_error = None; + cx.notify(); + return; + } + Ok(false) => {} + Err(message) => { + self.download_action_error = Some(message); + cx.notify(); + return; + } + } + + self.execute_download_file_action(download_id, action, cx); + } + + fn queue_security_confirmation( + &mut self, + download_id: &DownloadId, + action: DownloadFileAction, + ) -> Result { + let core = match &self.state { + ShellState::Ready(core) => core, + ShellState::StartupError(message) => return Err(message.clone()), + }; + + if !core + .download_requires_security_confirmation(download_id) + .map_err(|error| error.to_string())? + { + return Ok(false); + } + + let file_name = core + .snapshot() + .map_err(|error| error.to_string())? + .download_entries + .into_iter() + .find(|entry| entry.id() == download_id) + .map(|entry| entry.file_name().to_string()) + .ok_or_else(|| format!("download not found: {download_id}"))?; + + self.download_security_confirmation = + Some(PendingDownloadFileAction::new(download_id.clone(), file_name, action)); + Ok(true) + } + + fn execute_download_file_action( + &mut self, + download_id: &DownloadId, + action: DownloadFileAction, + cx: &mut Context, + ) { + let result = match &self.state { + ShellState::Ready(core) => core + .download_target_file_path(download_id) + .map_err(|error| error.to_string()) + .and_then(|path| action.run(&path).map_err(|error| error.to_string())), + ShellState::StartupError(message) => Err(message.clone()), + }; + + self.download_action_error = result.err(); + cx.notify(); + } +} diff --git a/crates/ely_app/src/shell/internal_pages/download_labels.rs b/crates/ely_app/src/shell/internal_pages/download_labels.rs index 8dfb063..df4d1ff 100644 --- a/crates/ely_app/src/shell/internal_pages/download_labels.rs +++ b/crates/ely_app/src/shell/internal_pages/download_labels.rs @@ -1,6 +1,4 @@ -use ely_domain::{ - DownloadDestination, DownloadEntry, DownloadPolicy, DownloadSecurity, DownloadState, -}; +use ely_domain::{DownloadDestination, DownloadEntry, DownloadPolicy, DownloadState}; pub(crate) fn download_state_label(state: &DownloadState) -> &'static str { match state { @@ -32,11 +30,11 @@ pub(crate) fn download_entry_location_label(entry: &DownloadEntry) -> String { } } -pub(crate) fn download_security_label(security: &DownloadSecurity) -> &'static str { - match security { - DownloadSecurity::Standard => "Standard", - DownloadSecurity::DangerousExtension => "Extension prompt required", +pub(crate) fn download_security_label(entry: &DownloadEntry) -> &'static str { + if entry.requires_security_confirmation() { + return "Confirm before opening"; } + "Extension confirmed" } fn download_destination_label(destination: &DownloadDestination) -> String { diff --git a/crates/ely_app/src/shell/internal_pages/downloads.rs b/crates/ely_app/src/shell/internal_pages/downloads.rs index fd1aec3..352665e 100644 --- a/crates/ely_app/src/shell/internal_pages/downloads.rs +++ b/crates/ely_app/src/shell/internal_pages/downloads.rs @@ -1,6 +1,6 @@ use ely_browser_core::BrowserSnapshot; use ely_design_system::colors; -use ely_domain::{DownloadChecksum, DownloadEntry, DownloadSecurity}; +use ely_domain::{DownloadChecksum, DownloadEntry}; use gpui::prelude::FluentBuilder; use gpui::{ AnyElement, Context, InteractiveElement, IntoElement, ParentElement, SharedString, Styled, div, @@ -12,6 +12,7 @@ use gpui_component::{ scroll::ScrollableElement, }; +use super::super::PendingDownloadFileAction; use super::{ ElyShell, download_labels::{ @@ -113,6 +114,9 @@ impl ElyShell { self.download_clear_confirmation && !snapshot.download_entries.is_empty(), |this| this.child(self.render_download_clear_confirmation(snapshot, cx)), ) + .when_some(self.download_security_confirmation.clone(), |this, pending_action| { + this.child(self.render_download_security_confirmation(&pending_action, cx)) + }) .child(self.render_downloads_list(snapshot, cx)), ) } @@ -166,6 +170,63 @@ impl ElyShell { .into_any_element() } + fn render_download_security_confirmation( + &mut self, + pending_action: &PendingDownloadFileAction, + cx: &mut Context, + ) -> AnyElement { + div() + .rounded_md() + .border_1() + .border_color(rgb(colors::ERROR)) + .px_3() + .py_2() + .flex() + .items_center() + .justify_between() + .gap_3() + .child( + div() + .min_w_0() + .flex() + .items_center() + .gap_2() + .text_xs() + .text_color(rgb(colors::ERROR)) + .child(IconName::TriangleAlert) + .child(div().truncate().child(format!( + "Confirm {} for {}", + pending_action.action_label(), + pending_action.file_name() + ))), + ) + .child( + div() + .flex() + .items_center() + .gap_2() + .child( + Button::new("cancel-security-download") + .ghost() + .xsmall() + .label("Cancel") + .on_click(cx.listener(|shell, _, _, cx| { + shell.cancel_download_security_prompt(cx); + })), + ) + .child( + Button::new("confirm-security-download") + .danger() + .xsmall() + .label("Confirm") + .on_click(cx.listener(|shell, _, _, cx| { + shell.confirm_download_security_prompt(cx); + })), + ), + ) + .into_any_element() + } + fn render_downloads_list( &mut self, snapshot: &BrowserSnapshot, @@ -258,7 +319,7 @@ impl ElyShell { .child(download_entry_location_label(entry)), ) .when(entry.security().requires_prompt(), |this| { - this.child(render_security_prompt(entry.security())) + this.child(render_security_prompt(entry)) }) .when_some(entry.checksum(), |this, checksum| { this.child(render_checksum_label(checksum)) @@ -316,14 +377,22 @@ fn render_download_action_error(message: String) -> AnyElement { .into_any_element() } -fn render_security_prompt(security: &DownloadSecurity) -> AnyElement { +fn render_security_prompt(entry: &DownloadEntry) -> AnyElement { + let prompt_color = + if entry.requires_security_confirmation() { colors::ERROR } else { colors::SUCCESS }; + let prompt_icon = if entry.requires_security_confirmation() { + IconName::TriangleAlert + } else { + IconName::CircleCheck + }; + div() .flex() .items_center() .gap_1() - .text_color(rgb(colors::ERROR)) - .child(IconName::TriangleAlert) - .child(download_security_label(security)) + .text_color(rgb(prompt_color)) + .child(prompt_icon) + .child(download_security_label(entry)) .into_any_element() } diff --git a/crates/ely_app/src/shell/mod.rs b/crates/ely_app/src/shell/mod.rs index 6431ad8..a017cf2 100644 --- a/crates/ely_app/src/shell/mod.rs +++ b/crates/ely_app/src/shell/mod.rs @@ -1,18 +1,18 @@ +mod downloads; mod internal_pages; mod render; use ely_browser_core::{BrowserCore, InitialBrowserConfig}; -use ely_domain::{CommandIntent, DownloadId, SpaceId, TabId, UrlText}; +use ely_domain::{CommandIntent, SpaceId, TabId, UrlText}; use gpui::{App, AppContext, Context, Entity, FocusHandle, Focusable, Subscription, Window}; use gpui_component::input::{InputEvent, InputState, SelectAll}; +use downloads::PendingDownloadFileAction; + use crate::{ CloseCurrentTab, FocusAddressBar, FocusCommandMode, OpenDownloads, OpenHistory, OpenNewTab, OpenSettings, RestoreClosedTab, SelectNextTab, SelectPreviousTab, ToggleFavoriteTab, TogglePinnedTab, - services::{ - download_checksums::DownloadChecksumCalculator, download_files::DownloadFileAction, - }, }; enum ShellState { @@ -27,6 +27,7 @@ pub struct ElyShell { last_intent: Option, download_action_error: Option, download_clear_confirmation: bool, + download_security_confirmation: Option, _command_subscription: Subscription, } @@ -84,6 +85,7 @@ impl ElyShell { last_intent: None, download_action_error: None, download_clear_confirmation: false, + download_security_confirmation: None, _command_subscription: command_subscription, } } @@ -222,102 +224,6 @@ impl ElyShell { } } - fn pause_download(&mut self, download_id: &DownloadId, cx: &mut Context) { - if let ShellState::Ready(core) = &mut self.state - && core.pause_download(download_id).is_ok() - { - cx.notify(); - } - } - - fn resume_download(&mut self, download_id: &DownloadId, cx: &mut Context) { - if let ShellState::Ready(core) = &mut self.state - && core.resume_download(download_id).is_ok() - { - cx.notify(); - } - } - - fn cancel_download(&mut self, download_id: &DownloadId, cx: &mut Context) { - if let ShellState::Ready(core) = &mut self.state - && core.cancel_download(download_id).is_ok() - { - cx.notify(); - } - } - - fn retry_download(&mut self, download_id: &DownloadId, cx: &mut Context) { - if let ShellState::Ready(core) = &mut self.state - && core.retry_download(download_id).is_ok() - { - cx.notify(); - } - } - - fn request_clear_active_profile_downloads(&mut self, cx: &mut Context) { - self.download_clear_confirmation = true; - cx.notify(); - } - - fn cancel_clear_active_profile_downloads(&mut self, cx: &mut Context) { - self.download_clear_confirmation = false; - cx.notify(); - } - - fn clear_active_profile_downloads(&mut self, cx: &mut Context) { - if let ShellState::Ready(core) = &mut self.state { - core.clear_downloads_for_active_profile(); - } - self.download_clear_confirmation = false; - self.download_action_error = None; - cx.notify(); - } - - fn calculate_download_checksum(&mut self, download_id: &DownloadId, cx: &mut Context) { - let result = match &mut self.state { - ShellState::Ready(core) => core - .download_target_file_path(download_id) - .map_err(|error| error.to_string()) - .and_then(|path| { - DownloadChecksumCalculator::sha256(&path).map_err(|error| error.to_string()) - }) - .and_then(|checksum| { - core.record_download_checksum(download_id, checksum) - .map_err(|error| error.to_string()) - }), - ShellState::StartupError(message) => Err(message.clone()), - }; - - self.download_action_error = result.err(); - cx.notify(); - } - - fn open_download_file(&mut self, download_id: &DownloadId, cx: &mut Context) { - self.run_download_file_action(download_id, DownloadFileAction::Open, cx); - } - - fn reveal_download_file(&mut self, download_id: &DownloadId, cx: &mut Context) { - self.run_download_file_action(download_id, DownloadFileAction::Reveal, cx); - } - - fn run_download_file_action( - &mut self, - download_id: &DownloadId, - action: DownloadFileAction, - cx: &mut Context, - ) { - let result = match &self.state { - ShellState::Ready(core) => core - .download_target_file_path(download_id) - .map_err(|error| error.to_string()) - .and_then(|path| action.run(&path).map_err(|error| error.to_string())), - ShellState::StartupError(message) => Err(message.clone()), - }; - - self.download_action_error = result.err(); - cx.notify(); - } - fn on_close_current_tab( &mut self, _: &CloseCurrentTab, diff --git a/crates/ely_browser_core/src/state/downloads.rs b/crates/ely_browser_core/src/state/downloads.rs index 44367b6..d22b75e 100644 --- a/crates/ely_browser_core/src/state/downloads.rs +++ b/crates/ely_browser_core/src/state/downloads.rs @@ -79,6 +79,21 @@ impl BrowserCore { Ok(()) } + pub fn confirm_download_security_prompt( + &mut self, + download_id: &DownloadId, + ) -> Result<(), CoreError> { + self.download_entry_mut(download_id)?.confirm_security_prompt()?; + Ok(()) + } + + pub fn download_requires_security_confirmation( + &self, + download_id: &DownloadId, + ) -> Result { + Ok(self.visible_download_entry(download_id)?.requires_security_confirmation()) + } + pub fn download_target_file_path( &self, download_id: &DownloadId, diff --git a/crates/ely_browser_core/tests/downloads.rs b/crates/ely_browser_core/tests/downloads.rs index 25baa00..aa26ef5 100644 --- a/crates/ely_browser_core/tests/downloads.rs +++ b/crates/ely_browser_core/tests/downloads.rs @@ -171,6 +171,63 @@ fn records_active_profile_download_policy_on_started_entry() -> Result<(), Box Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let download_id = core.record_download_started( + UrlText::parse("https://example.com/installer.dmg")?, + "installer.dmg", + Some(4096), + )?; + + core.complete_download(&download_id, 4096)?; + assert!(core.download_requires_security_confirmation(&download_id)?); + core.confirm_download_security_prompt(&download_id)?; + assert!(!core.download_requires_security_confirmation(&download_id)?); + assert!(active_download(&core)?.security_prompt_confirmed()); + Ok(()) +} + +#[test] +fn rejects_security_prompt_confirmation_before_completion() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let download_id = core.record_download_started( + UrlText::parse("https://example.com/installer.dmg")?, + "installer.dmg", + Some(4096), + )?; + + let error = match core.confirm_download_security_prompt(&download_id) { + Ok(()) => return Err("security prompt confirmation should require completion".into()), + Err(error) => error, + }; + + assert_eq!( + error, + CoreError::Domain(DomainError::InvalidDownloadTransition { + action: "confirm security prompt", + state: "in_progress" + }) + ); + Ok(()) +} + +#[test] +fn standard_download_skips_security_confirmation() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let download_id = core.record_download_started( + UrlText::parse("https://example.com/report.pdf")?, + "report.pdf", + Some(2048), + )?; + + core.complete_download(&download_id, 2048)?; + assert!(!core.download_requires_security_confirmation(&download_id)?); Ok(()) } diff --git a/crates/ely_domain/src/download.rs b/crates/ely_domain/src/download.rs index 5af9a39..b38eae1 100644 --- a/crates/ely_domain/src/download.rs +++ b/crates/ely_domain/src/download.rs @@ -55,6 +55,7 @@ pub struct DownloadEntry { received_bytes: u64, total_bytes: Option, checksum: Option, + security_prompt_confirmed: bool, started_at: SystemTime, } @@ -212,6 +213,7 @@ impl DownloadEntry { received_bytes: 0, total_bytes, checksum: None, + security_prompt_confirmed: false, started_at, }) } @@ -268,6 +270,12 @@ impl DownloadEntry { Ok(()) } + pub fn confirm_security_prompt(&mut self) -> Result<(), DomainError> { + self.require_state("confirm security prompt", &[DownloadState::Completed])?; + self.security_prompt_confirmed = true; + Ok(()) + } + #[must_use] pub fn id(&self) -> &DownloadId { &self.id @@ -303,6 +311,16 @@ impl DownloadEntry { &self.security } + #[must_use] + pub fn requires_security_confirmation(&self) -> bool { + self.security.requires_prompt() && !self.security_prompt_confirmed + } + + #[must_use] + pub fn security_prompt_confirmed(&self) -> bool { + self.security_prompt_confirmed + } + #[must_use] pub fn state(&self) -> &DownloadState { &self.state