diff --git a/ROADMAP.md b/ROADMAP.md index ecad8c6..b0e3003 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-12 | Security: cell hyperlinks open only http, https and mailto, after a confirmation; other schemes (file:, smb:, UNC, custom handlers) are refused (#777, part 1) | | 2026-10-12 | Deleting several Ctrl-selected rows or columns removes them all in one undo step (#758) | | 2026-10-12 | XLSX chart sheets are kept on open and save: the sheet, its drawing and chart (with style and colours) are written back unchanged in their place among the worksheets, and left out when the sheet they plot is deleted or renamed or a worksheet took their name (#727) | | 2026-10-12 | CSV and tab-separated export is capped at 512 MiB: a sheet whose used range would expand to more (two cells far apart, such as A1 and XFD1048576, are 16 GiB of commas) is refused with the rows, columns and limit, before anything is allocated or written (#528) | diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index 3856aaf..c0fcb22 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -170,8 +170,10 @@ fn services(inbox: Inbox, ctx: egui::Context) -> Services { } })), open_url: Some(Box::new(|url: &str| { - if let Some(w) = web_sys::window() { - let _ = w.open_with_url_and_target(url, "_blank"); + if let Some(w) = web_sys::window() + && let gridcraft_engine::links::LinkKind::External(url) = gridcraft_engine::links::classify(url) + { + let _ = w.open_with_url_and_target(&url, "_blank"); } })), inbox: Some(inbox), diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs index 08737a3..7c0eb04 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -201,6 +201,13 @@ fn services() -> Services { /// Opens a URL or file with the system's default application. fn open_with_system(target: &str) { + // Last line of defence behind `link.open` and the click prompt: the system only ever gets + // http, https and mailto addresses. + let gridcraft_engine::links::LinkKind::External(target) = gridcraft_engine::links::classify(target) else { + log::warn!("refused to open {target:?}: only http, https and mailto links are opened"); + return; + }; + let target = target.as_str(); let cmd = if cfg!(target_os = "macos") { "open" } else if cfg!(windows) { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 0b467e0..82dbe0b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -503,7 +503,7 @@ fn select_col(s: &mut Session, _: &Json) -> Result { Ok(json!({"selection": d.selection.a1()})) } -fn go_to(s: &mut Session, p: &Json) -> Result { +pub(super) fn go_to(s: &mut Session, p: &Json) -> Result { let Some(reference) = str_param(p, "reference").or_else(|| str_param(p, "range")) else { return Err(bad("edit.goTo", "missing `reference`")) }; // A defined name or table? let d = s.doc()?; diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index e589e60..3199542 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -157,6 +157,15 @@ pub fn specs() -> Vec { has_doc, insert_link ), + cmd!( + "link.open", + "Open Hyperlink", + [], + None, + "{url?: \"https://…\" | \"mailto:…\" | \"Sheet2!A1\", cell?} (default: the cell's link); only http, https and mailto are opened, any other scheme is an error; the UI asks before opening", + has_doc, + open_link + ), cmd!("review.newNote", "New Note", ["Review", "Notes"], Some("Shift+F2"), "{cell?, text, author?}", has_doc, |s, p| comment(s, p, false)), cmd!("review.newComment", "New Comment", ["Review", "Comments"], Some("Cmd+Shift+F2"), "{cell?, text, author?}", has_doc, |s, p| comment( s, p, true @@ -1184,6 +1193,31 @@ fn insert_link(s: &mut Session, p: &Json) -> Result { }) } +/// Follows a link: a reference selects it, an `http`/`https`/`mailto` address is handed to the +/// host (`UiRequest::OpenUrl`); anything else is refused ([`crate::links::classify`]). Opens no +/// dialog; the interactive click asks for confirmation first, in the UI. +fn open_link(s: &mut Session, p: &Json) -> Result { + let target = match str_param(p, "url").or_else(|| str_param(p, "target")) { + Some(t) => t.to_string(), + None => { + let sheet = target_sheet(s, p)?; + let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); + let Some(h) = s.doc()?.wb.sheet(sheet).and_then(|sh| sh.hyperlinks.get(&at)) else { + return Err(EngineError::Other("There's no hyperlink in this cell.".into())); + }; + h.target.clone() + } + }; + match crate::links::classify(&target) { + crate::links::LinkKind::Internal(r) => super::edit::go_to(s, &json!({"reference": r.trim_start_matches('#')})), + crate::links::LinkKind::External(u) => { + s.ui_requests.push(crate::UiRequest::OpenUrl(u.clone())); + Ok(json!({"opened": u})) + } + crate::links::LinkKind::Refused(why) => Err(EngineError::Other(why)), + } +} + /// Sorts a link target as the two pages of Excel's Insert Hyperlink dialog do: "Place in This /// Document" is a sheet reference, a defined name or `#…`; anything else ("Existing File or Web /// Page") is outside the workbook and saved as a relationship. diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5a2e3df..258a1cb 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -13,6 +13,7 @@ pub mod cmd; pub mod display; pub mod fill; pub mod io; +pub mod links; pub mod outline; pub mod paper; pub mod pivot; diff --git a/crates/engine/src/links.rs b/crates/engine/src/links.rs new file mode 100644 index 0000000..04d8da6 --- /dev/null +++ b/crates/engine/src/links.rs @@ -0,0 +1,83 @@ +//! Which hyperlink targets may be opened. A cell's link comes from the document (often a file +//! somebody sent), so it is hostile input: only `http`, `https` and `mailto` ever leave the +//! workbook for the system; `file:`, `smb:`, UNC paths, custom URL handlers, `javascript:` and +//! the like are refused. Everything without a scheme stays an in-workbook reference. + +use gridcraft_core::RangeRef; + +/// What a link target is. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum LinkKind { + /// A cell, range, name or table of the workbook (`Sheet2!A1`, `Total`). + Internal(String), + /// An address to hand to the system, trimmed: `http`, `https` or `mailto` only. + External(String), + /// Refused, with the reason to show. + Refused(String), +} + +const ALLOWED: [&str; 3] = ["http", "https", "mailto"]; + +/// Classifies a link target. Never panics; the single place that decides what may be opened. +pub fn classify(target: &str) -> LinkKind { + let t = target.trim(); + if t.chars().any(char::is_control) { + return LinkKind::Refused("The link contains control characters.".into()); + } + if t.starts_with("\\\\") || t.starts_with("//") { + return LinkKind::Refused("Links to network locations aren't opened.".into()); + } + if RangeRef::parse(t).is_some() { + return LinkKind::Internal(t.to_string()); + } + let Some((scheme, rest)) = t.split_once(':') else { return LinkKind::Internal(t.to_string()) }; + let scheme_like = scheme.chars().next().is_some_and(|c| c.is_ascii_alphabetic()) + && scheme.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')); + if !scheme_like { + return LinkKind::Internal(t.to_string()); + } + let lower = scheme.to_ascii_lowercase(); + if ALLOWED.contains(&lower.as_str()) && !rest.trim().is_empty() { + LinkKind::External(t.to_string()) + } else { + LinkKind::Refused(format!("Links of type \"{lower}:\" aren't opened. Only http, https and mailto links are allowed.")) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_http_https_and_mailto_leave_the_workbook() { + for ok in ["http://a.example/x", "HTTPS://A.example", " https://a.example ", "MailTo:a@b.example"] { + assert!(matches!(classify(ok), LinkKind::External(_)), "{ok}"); + } + assert_eq!(classify(" https://a.example "), LinkKind::External("https://a.example".into())); + for bad in [ + "javascript:alert(1)", + "JaVaScRiPt:alert(1)", + "file:///etc/passwd", + "smb://host/share", + "ms-excel:ofe|u|http://x", + "\\\\host\\share\\x.exe", + "//host/share", + "C:\\Windows\\calc.exe", + "\tjavascript:1", + "java\nscript:1", + "\u{1}http://a.example", + "http:", + "mailto:", + "x-custom://a", + ] { + assert!(matches!(classify(bad), LinkKind::Refused(_)), "{bad:?} -> {:?}", classify(bad)); + } + } + + #[test] + fn references_stay_internal() { + for r in ["Sheet2!A1", "'My Sheet'!B2:C3", "A1:B2", "#Sheet1!A1", "Total", "", " "] { + assert!(matches!(classify(r), LinkKind::Internal(_)), "{r:?} -> {:?}", classify(r)); + } + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0de5681..6f4b67a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3141,6 +3141,22 @@ fn a_decrypted_workbook_never_overwrites_its_encrypted_original() { std::fs::remove_dir_all(dir).unwrap(); } +#[test] +fn link_open_refuses_every_scheme_but_http_https_and_mailto() { + let mut s = s(); + s.execute("insert.link", json!({"cell": "A1", "target": "file:///etc/passwd"})).unwrap(); + s.take_ui_requests(); + for url in ["file:///etc/passwd", "smb://h/s", "\\\\h\\s", "JAVASCRIPT:1"] { + assert!(s.execute("link.open", json!({"url": url})).is_err(), "{url}"); + } + assert!(s.execute("link.open", json!({"cell": "A1"})).is_err()); + assert!(s.take_ui_requests().is_empty()); + s.execute("link.open", json!({"url": "HTTPS://example.org/a"})).unwrap(); + assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl("HTTPS://example.org/a".into())]); + s.execute("link.open", json!({"url": "B2"})).unwrap(); + assert!(s.take_ui_requests().is_empty()); +} + #[test] fn subtotal_109_recalculates_when_row_height_or_autofit_reveals_rows() { let mut s = s(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index b98a4d5..0cc4ac4 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -349,6 +349,20 @@ impl Dialog { json!({"text": existing}), ) } + // Asks before a web or e-mail link leaves the workbook; OK runs `link.open`, which + // refuses anything but http, https and mailto. + "openLink" => { + let url = params.get("url").and_then(Json::as_str).unwrap_or_default().to_string(); + Dialog::form( + "openLink", + "Open Hyperlink", + "link.open", + vec![Note(format!( + "This workbook asks to open the address below. Links can lead to harmful sites or start other programs: open it only if you trust the workbook.\n\n{url}" + ))], + json!({"url": url}), + ) + } "insertLink" => Dialog::form( "insertLink", "Insert Hyperlink", diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 13f5f28..e52a0fc 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2459,15 +2459,18 @@ fn delete_selected_object(app: &mut SheetApp, sh: &Sheet) -> bool { true } +/// A click on a hyperlink. References select; web and e-mail addresses ask first (the dialog's +/// OK runs `link.open`); every other kind is refused with a message. The decision is the +/// engine's ([`gridcraft_engine::links::classify`]), the same one `link.open` applies. pub fn follow_link(app: &mut SheetApp, target: &str) { - if target.contains("://") || target.starts_with("mailto:") { - if let Some(f) = &app.services.open_url { - f(target); + use gridcraft_engine::links::{LinkKind, classify}; + match classify(target) { + LinkKind::Internal(r) => { + let _ = app.session.run("edit.goTo", json!({"reference": r.trim_start_matches('#')})); + app.grid.ensure_visible = true; } - } else { - let t = target.trim_start_matches('#'); - let _ = app.session.run("edit.goTo", json!({"reference": t})); - app.grid.ensure_visible = true; + LinkKind::External(url) => app.open_dialog("openLink", json!({"url": url})), + LinkKind::Refused(why) => app.message = Some(("Hyperlink".into(), why)), } }