chore: harden URL opening

This commit is contained in:
Tomás Romero committed 2026-10-17 23:33:07 +02:00
1 parent 7ea2a02974
commit efd1554208
8 files changed
+248 -52

No files matched your search

+18
View File
@@ -51,6 +51,7 @@ host-provided service:
| `documents` | Native file dialogs | Implements `documents::DocumentProvider` with the system picker |
| `credentials` | System keychain (`SystemCredentialStore`) | Implements `CredentialStore` with the platform keystore |
| `oauth_redirects` | Loopback redirect broker | Implements `OAuthRedirectBroker` (app links, custom schemes) |
| `external_urls` | System URL launcher | `ExternalUrlOpener::new` with the host's native URL bridge |
Mobile hosts use `PlatformContext::app_private(data_root, cache_dir, documents,
credentials, oauth_redirects)`, which keeps durable data below
@@ -58,6 +59,23 @@ credentials, oauth_redirects)`, which keeps durable data below
`UnavailableCredentialStore` so account setup reports the gap. Never fall back
to a plaintext store.
The constructors default `external_urls` to the system URL launcher. A host
without desktop launchers configures its context before calling `run`:
```rust,ignore
platform.external_urls = flectar_mail::ExternalUrlOpener::new(bridge::open_url);
```
This replaces the process-global `set_external_url_opener` registration. The
callback may capture host state; cloned contexts share it. Mail links invoke it
on the application event loop, and browser-based OAuth (including retries)
invokes it on a worker thread. The host must marshal calls onto any thread its
native API requires and return promptly after submitting the request. Errors
reach the UI without falling back to a desktop launcher. Email links retain
their `http`/`https`/`mailto`/`tel` allowlist, while browser authorization requires
a parsed HTTPS URL with a host. The existing in-app `mailto` compose route and
native OAuth broker flows remain in place.
Each mobile operating system also has a module in `src/platform/` that
provides these crate-internal items, which shared code calls without a `cfg`:
+147 -33
View File
@@ -1,18 +1,50 @@
//! Open the system browser without leaking AppImage libraries into host programs.
//! Host URL dispatch, with separate policies for mail links and authorization.
use std::sync::OnceLock;
use std::sync::Arc;
/// Signature of a platform-installed opener for external addresses.
type ExternalUrlOpener = fn(&str) -> Result<(), String>;
type OpenUrl = dyn Fn(&str) -> Result<(), String> + Send + Sync;
/// Opener installed by a platform integration that cannot hand addresses to a
/// helper process (for example, a host-managed sandbox without
/// `xdg-open`-style tools). The first registration wins.
static EXTERNAL_OPENER: OnceLock<ExternalUrlOpener> = OnceLock::new();
/// The URL service carried by [`crate::PlatformContext`].
///
/// The default uses the system browser, preserving Linux's portal and AppImage
/// library-path handling for authorization. Hosts without desktop launchers
/// supply a callback with [`Self::new`]. Clones share that callback's state.
#[derive(Clone, Default)]
pub struct ExternalUrlOpener {
handler: Option<Arc<OpenUrl>>,
}
/// Install the opener used by [`open_external`].
pub fn set_external_url_opener(opener: fn(&str) -> Result<(), String>) {
let _ = EXTERNAL_OPENER.set(opener);
impl ExternalUrlOpener {
/// Use a host callback for both external links and browser authorization.
///
/// Mail links call this on the application event loop; authorization calls
/// it on a worker thread. Hosts must marshal native API calls to their
/// required thread. Return promptly after handing the URL to the host,
/// without waiting for the browser to close. Errors are shown to the user;
/// a failed host callback never falls back to a desktop launcher.
pub fn new(opener: impl Fn(&str) -> Result<(), String> + Send + Sync + 'static) -> Self {
Self {
handler: Some(Arc::new(opener)),
}
}
pub(super) fn open_external(&self, url: &str) -> Result<(), String> {
match &self.handler {
Some(opener) => opener(url),
None => webbrowser::open(url).map_err(|error| error.to_string()),
}
}
pub(super) fn open_authorization(&self, url: &str) -> Result<(), String> {
let address = url::Url::parse(url).map_err(|_| "Sign-in address is invalid".to_owned())?;
if address.scheme() != "https" || address.host_str().is_none() {
return Err("Sign-in requires an HTTPS address".into());
}
match &self.handler {
Some(opener) => opener(address.as_str()),
None => open_system_authorization(&address),
}
}
}
#[cfg(linux_desktop)]
@@ -90,19 +122,14 @@ fn open_with_desktop_portal(address: &url::Url) -> Result<(), String> {
})
}
pub(super) fn open(url: &str) -> Result<(), String> {
// This entry point is only for provider authorization, never a shell command.
let address = url::Url::parse(url).map_err(|_| "Sign-in address is invalid".to_owned())?;
if address.scheme() != "https" || address.host_str().is_none() {
return Err("Sign-in requires an HTTPS address".into());
}
fn open_system_authorization(address: &url::Url) -> Result<(), String> {
let url = address.as_str();
#[cfg(linux_desktop)]
{
// The portal is the reliable route from AppImages and desktop
// containers to the user's host browser. Keep command-based fallbacks
// for minimal desktops that do not run xdg-desktop-portal.
if open_with_desktop_portal(&address).is_ok() {
if open_with_desktop_portal(address).is_ok() {
return Ok(());
}
let mut xdg = host_command("xdg-open");
@@ -118,16 +145,109 @@ pub(super) fn open(url: &str) -> Result<(), String> {
webbrowser::open(url).map_err(|e| e.to_string())
}
/// Open an address that only needs to reach the system browser: links clicked
/// in a mail, project pages. Unlike [`open`] — the provider authorization
/// entry point, which insists on a validated HTTPS sign-in address — this
/// passes the address through as-is and prefers a platform-installed opener
/// when one exists.
pub(super) fn open_external(url: &str) -> Result<(), String> {
if let Some(opener) = EXTERNAL_OPENER.get() {
return opener(url);
#[cfg(test)]
mod dispatch_tests {
use super::*;
use crate::mail_render_projection::open_email_link;
use std::sync::Mutex;
#[test]
fn mail_links_preserve_allowed_addresses_and_reject_other_schemes() {
let addresses = Arc::new(Mutex::new(Vec::new()));
let recorded = addresses.clone();
let opener = ExternalUrlOpener::new(move |url| {
recorded.lock().unwrap().push(url.to_owned());
Ok(())
});
let allowed = [
"http://example.com/path?q=1#part",
"https://example.com/a%20b?x=1&y=2",
"HTTPS://example.com/",
"mailto:person@example.com?subject=Hello%20there",
"tel:+123456789",
];
for url in allowed {
open_email_link(&opener, &format!(" {url}\n")).unwrap();
}
for url in [
"",
"example.com",
"/relative/path",
"#fragment",
"--help",
"javascript:alert(1)",
"data:text/html,hello",
"file:///tmp/message.html",
"intent://example.com",
] {
assert!(open_email_link(&opener, url).is_err(), "{url}");
}
assert_eq!(*addresses.lock().unwrap(), allowed);
}
#[test]
fn authorization_validates_before_dispatch_for_default_and_custom_openers() {
let custom = ExternalUrlOpener::new(|_| panic!("invalid authorization reached the host"));
for opener in [ExternalUrlOpener::default(), custom] {
for url in [
"http://example.com/",
"file:///tmp/test",
"mailto:person@example.com",
"javascript:alert(1)",
"--help",
"https://",
"https://[invalid]/",
"/relative/path",
] {
assert!(opener.open_authorization(url).is_err(), "{url}");
}
}
}
#[test]
fn authorization_and_mail_share_the_host_across_threads() {
let addresses = Arc::new(Mutex::new(Vec::new()));
let recorded = addresses.clone();
let opener = ExternalUrlOpener::new(move |url| {
recorded.lock().unwrap().push(url.to_owned());
Ok(())
});
open_email_link(&opener, "https://example.com/mail").unwrap();
let worker_opener = opener.clone();
std::thread::spawn(move || {
worker_opener
.open_authorization("HTTPS://EXAMPLE.COM/sign-in?state=a%2Bb&code_challenge=c")
})
.join()
.unwrap()
.unwrap();
assert_eq!(
*addresses.lock().unwrap(),
[
"https://example.com/mail",
"https://example.com/sign-in?state=a%2Bb&code_challenge=c",
]
);
}
#[test]
fn host_errors_propagate_without_fallback_or_global_registration() {
let first = ExternalUrlOpener::new(|_| Err("first host unavailable".into()));
let second = ExternalUrlOpener::new(|_| Err("second host unavailable".into()));
for (opener, expected) in [
(first, "first host unavailable"),
(second, "second host unavailable"),
] {
assert_eq!(
open_email_link(&opener, "https://example.com/"),
Err(expected.to_owned())
);
assert_eq!(
opener.open_authorization("https://example.com/"),
Err(expected.to_owned())
);
}
}
webbrowser::open(url).map_err(|error| error.to_string())
}
#[cfg(all(test, linux_desktop))]
@@ -146,12 +266,6 @@ mod tests {
assert!(launch(command).is_ok());
}
#[test]
fn rejects_non_https_authorization() {
assert!(open("file:///tmp/test").is_err());
assert!(open("--help").is_err());
assert!(open("https://").is_err());
}
#[test]
fn restores_host_libraries_only_for_bundled_builds() {
let mut command = Command::new("xdg-open");
restore_library_path(&mut command, false, None);
+2 -2
View File
@@ -384,13 +384,13 @@ pub(super) fn apply_cpu_frame(app: &AppWindow, rendered: RenderedEmail) {
}
}
pub(super) fn open_email_link(url: &str) -> Result<(), String> {
pub(super) fn open_email_link(opener: &ExternalUrlOpener, url: &str) -> Result<(), String> {
let Some((scheme, _)) = url.trim().split_once(':') else {
return Err("only absolute links can be opened".into());
};
match scheme.to_ascii_lowercase().as_str() {
"http" | "https" | "mailto" | "tel" => {
crate::browser::open_external(url.trim())?;
opener.open_external(url.trim())?;
Ok(())
}
_ => Err("unsupported link type".into()),
+25 -6
View File
@@ -4,7 +4,7 @@ mod account_mail_preferences;
mod account_removal_tests;
mod attachment_controller;
mod browser;
pub use browser::set_external_url_opener;
pub use browser::ExternalUrlOpener;
mod calendar;
mod compose_controller;
mod compose_editor;
@@ -1082,6 +1082,8 @@ impl InboxState {
/// sandbox paths and native services. See platform/README.md.
#[derive(Clone)]
pub struct PlatformContext {
/// Host URL dispatch for mail links and browser-based authorization.
pub external_urls: ExternalUrlOpener,
pub paths: Paths,
pub documents: Arc<dyn documents::DocumentProvider>,
pub credentials: flectar_mail_core::accounts::credentials::CredentialStoreHandle,
@@ -1095,6 +1097,7 @@ impl PlatformContext {
let credentials = desktop_credential_store(&paths);
Ok(Self {
paths,
external_urls: ExternalUrlOpener::default(),
documents: documents::default_provider(),
credentials,
oauth_redirects: Arc::new(
@@ -1114,6 +1117,7 @@ impl PlatformContext {
) -> Self {
Self {
paths: Paths::new(data_root.join("data"), cache_dir),
external_urls: ExternalUrlOpener::default(),
documents,
credentials,
oauth_redirects,
@@ -6239,7 +6243,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
let state_for_account = Rc::clone(&state);
let runtime_for_account = Rc::clone(&runtime);
let ui_task_tx_for_account = ui_task_tx.clone();
oauth_browser::register(&app, platform.oauth_redirects.clone());
oauth_browser::register(
&app,
platform.oauth_redirects.clone(),
platform.external_urls.clone(),
);
mail_setup::register(&app, &runtime);
let suggestion_app = app.as_weak();
@@ -6433,6 +6441,7 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
let state_for_oauth = Rc::clone(&state);
let runtime_for_oauth = Rc::clone(&runtime);
let ui_task_tx_for_oauth = ui_task_tx.clone();
let opener_for_oauth = platform.external_urls.clone();
app.on_start_oauth(move |provider| {
let Some(app) = app_weak.upgrade() else {
return;
@@ -6473,10 +6482,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
));
let updates = ui_task_tx_for_oauth.clone();
let browser_app = app.as_weak();
let opener = opener_for_oauth.clone();
runtime_for_oauth.spawn(async move {
let result = core
.start_oauth_with_calendar(provider, connect_calendar, move |url| {
oauth_browser::begin(browser_app.clone(), url)
oauth_browser::begin(browser_app.clone(), url, opener.clone())
})
.await;
let update = match result {
@@ -6519,6 +6529,7 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
let state_for_reauth = Rc::clone(&state);
let runtime_for_reauth = Rc::clone(&runtime);
let ui_task_tx_for_reauth = ui_task_tx.clone();
let opener_for_reauth = platform.external_urls.clone();
app.on_reauth_account(move |account_id| {
let Some(app) = app_weak.upgrade() else {
return;
@@ -6546,10 +6557,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
));
let updates = ui_task_tx_for_reauth.clone();
let browser_app = app.as_weak();
let opener = opener_for_reauth.clone();
runtime_for_reauth.spawn(async move {
let result = core
.reauth_account(i64::from(account_id), move |url| {
oauth_browser::begin(browser_app.clone(), url)
oauth_browser::begin(browser_app.clone(), url, opener.clone())
})
.await;
let update = match result {
@@ -6604,6 +6616,7 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
let state_for_calendar_connect = Rc::clone(&state);
let runtime_for_calendar_connect = Rc::clone(&runtime);
let updates_for_calendar_connect = ui_task_tx.clone();
let opener_for_calendar_connect = platform.external_urls.clone();
app.on_connect_account_calendar(move |account_id| {
let Some(app) = app_weak.upgrade() else {
return;
@@ -6642,10 +6655,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
));
let updates = updates_for_calendar_connect.clone();
let browser_app = app.as_weak();
let opener = opener_for_calendar_connect.clone();
runtime_for_calendar_connect.spawn(async move {
let result = core
.connect_provider_calendar(i64::from(account_id), provider, move |url| {
oauth_browser::begin(browser_app.clone(), url)
oauth_browser::begin(browser_app.clone(), url, opener.clone())
})
.await;
let connections = core.load_calendar_connections().await.ok();
@@ -6899,7 +6913,12 @@ pub fn run(platform: PlatformContext) -> Result<(), Box<dyn std::error::Error>>
&calendar_state,
);
register_renderer_input_callbacks(&app, &email_renderer, use_wgpu);
register_renderer_input_callbacks(
&app,
&email_renderer,
use_wgpu,
platform.external_urls.clone(),
);
let app_weak = app.as_weak();
let state_for_search = Rc::clone(&state);
+28 -6
View File
@@ -1,7 +1,11 @@
//! Keep the redirect listener alive when a desktop browser cannot be launched.
use super::*;
pub(super) fn begin(app: slint::Weak<AppWindow>, url: String) -> Result<(), String> {
pub(super) fn begin(
app: slint::Weak<AppWindow>,
url: String,
opener: ExternalUrlOpener,
) -> Result<(), String> {
let link = url.clone();
app.upgrade_in_event_loop(move |app| {
if app.get_oauth_in_progress() {
@@ -10,13 +14,13 @@ pub(super) fn begin(app: slint::Weak<AppWindow>, url: String) -> Result<(), Stri
}
})
.map_err(|e| e.to_string())?;
launch(app, url);
launch(app, url, opener);
Ok(())
}
fn launch(app: slint::Weak<AppWindow>, url: String) {
fn launch(app: slint::Weak<AppWindow>, url: String, opener: ExternalUrlOpener) {
std::thread::spawn(move || {
let failed = browser::open(&url).is_err();
let failed = opener.open_authorization(&url).is_err();
let _ = app.upgrade_in_event_loop(move |app| {
if app.get_oauth_in_progress() && app.get_oauth_authorization_url().as_str() == url {
app.set_oauth_browser_error(failed);
@@ -28,6 +32,7 @@ fn launch(app: slint::Weak<AppWindow>, url: String) {
pub(super) fn register(
app: &AppWindow,
redirects: flectar_mail_core::oauth::redirect::OAuthRedirectBrokerHandle,
opener: ExternalUrlOpener,
) {
let weak = app.as_weak();
app.on_retry_oauth_browser(move || {
@@ -36,7 +41,7 @@ pub(super) fn register(
};
let url = app.get_oauth_authorization_url().to_string();
if app.get_oauth_in_progress() && !url.is_empty() {
launch(app.as_weak(), url);
launch(app.as_weak(), url, opener.clone());
}
});
let weak = app.as_weak();
@@ -83,15 +88,21 @@ mod tests {
}
}
#[test]
fn onboarding_copies_active_link_without_a_mailbox_and_ignores_finished_sign_in() {
fn onboarding_copies_and_retries_active_link_and_ignores_finished_sign_in() {
let window = MinimalSoftwareWindow::new(RepaintBufferType::NewBuffer);
let clipboard = Rc::new(RefCell::new(String::new()));
slint::platform::set_platform(Box::new(Headless(window.clone(), clipboard.clone())))
.unwrap();
let app = AppWindow::new().unwrap();
let (opened, received) = std::sync::mpsc::channel();
let opener = ExternalUrlOpener::new(move |url| {
opened.send(url.to_owned()).unwrap();
Err("test host unavailable".into())
});
register(
&app,
Arc::new(flectar_mail_core::oauth::redirect::LoopbackRedirectBroker::default()),
opener.clone(),
);
app.set_startup_ready(true);
app.set_oauth_in_progress(true);
@@ -106,10 +117,21 @@ mod tests {
*clipboard.borrow(),
app.get_oauth_authorization_url().as_str()
);
assert!(
received.try_recv().is_err(),
"copy must not launch a browser"
);
let url = app.get_oauth_authorization_url().to_string();
launch(app.as_weak(), url.clone(), opener);
assert_eq!(received.recv_timeout(Duration::from_secs(5)).unwrap(), url);
app.invoke_retry_oauth_browser();
assert_eq!(received.recv_timeout(Duration::from_secs(5)).unwrap(), url);
app.set_oauth_in_progress(false);
app.set_oauth_authorization_url("".into());
*clipboard.borrow_mut() = "keep clipboard".into();
app.invoke_copy_oauth_link();
app.invoke_retry_oauth_browser();
assert_eq!(*clipboard.borrow(), "keep clipboard");
assert!(received.try_recv().is_err());
}
}
+2 -3
View File
@@ -23,6 +23,7 @@ pub fn launch_context() -> Result<crate::PlatformContext, flectar_mail_core::err
)
})?;
Ok(crate::PlatformContext {
external_urls: crate::ExternalUrlOpener::default(),
paths: flectar_mail_core::config::Paths::new(
root.join("Library/Application Support/Flectar"),
root.join("Library/Caches/Flectar"),
@@ -30,9 +31,7 @@ pub fn launch_context() -> Result<crate::PlatformContext, flectar_mail_core::err
documents: Arc::new(documents::IosDocuments),
// The Keychain bridge is not integrated yet; account setup reports
// that instead of storing secrets anywhere else.
credentials: Arc::new(
flectar_mail_core::accounts::credentials::UnavailableCredentialStore,
),
credentials: Arc::new(flectar_mail_core::accounts::credentials::UnavailableCredentialStore),
oauth_redirects: Arc::new(
flectar_mail_core::oauth::redirect::LoopbackRedirectBroker::default(),
),
+24 -1
View File
@@ -69,7 +69,30 @@ fn automatic_selection_hydrates_and_reader_controls_are_responsive_and_keyboard_
.borrow_mut()
.configure_resources(runtime.handle().clone(), false)
.unwrap();
register_renderer_input_callbacks(&app, &renderer, false);
let opened_urls = Arc::new(std::sync::Mutex::new(Vec::new()));
let recorded_urls = opened_urls.clone();
register_renderer_input_callbacks(
&app,
&renderer,
false,
ExternalUrlOpener::new(move |url| {
recorded_urls.lock().unwrap().push(url.to_owned());
Err("test host unavailable".into())
}),
);
app.invoke_open_email_link("https://example.com/external".into());
assert_eq!(
*opened_urls.lock().unwrap(),
["https://example.com/external"]
);
assert_eq!(app.get_render_message().key, "Could not open link: {}");
assert_eq!(app.get_render_message().argument, "test host unavailable");
app.invoke_open_email_link("javascript:alert(1)".into());
app.invoke_open_email_link("#fragment".into());
app.invoke_open_email_link("mailto:person@example.com?subject=Hello%20there".into());
assert_eq!(app.get_compose_to(), "person@example.com");
assert_eq!(app.get_compose_subject(), "Hello there");
assert_eq!(opened_urls.lock().unwrap().len(), 1);
let requests = Rc::new(RefCell::new(Vec::new()));
let requests2 = requests.clone();
app.global::<EmailReader>()
+2 -1
View File
@@ -78,6 +78,7 @@ pub(super) fn register_renderer_input_callbacks(
app: &AppWindow,
renderer: &Rc<RefCell<GpuEmailRenderer>>,
gpu: bool,
opener: ExternalUrlOpener,
) {
let weak = app.as_weak();
let appearance_renderer = renderer.clone();
@@ -136,7 +137,7 @@ pub(super) fn register_renderer_input_callbacks(
}
return;
}
if let Err(error) = open_email_link(&url) {
if let Err(error) = open_email_link(&opener, &url) {
app.set_render_status(UiMessage::detail("Could not open link: {}", error));
}
});