diff --git a/ROADMAP.md b/ROADMAP.md index e8e1c06..663da50 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 | File › Print selects a system printer, copies, duplex and grayscale, submits PDF jobs through craft-print on a worker and shows spooler errors; browser printing and PDF export remain available | | 2026-10-11 | Command-sequence fuzzing: random command sequences checked against undo/redo, XLSX round-trip and full recalculation; fixes for the table, legacy-array, spill, AutoSum, Watch Window and sheet-copy bugs it found | | 2026-10-11 | Password-protected workbooks open: GridCraft asks for the password and decrypts Agile and Standard encryption (#11) | diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index e1350a1..d07aad4 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -144,8 +144,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 8093205..b48ed90 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -194,6 +194,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 3da230b..90bc203 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -513,7 +513,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 392c5ee..284ba3a 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 @@ -1173,6 +1182,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)), + } +} + fn comment(s: &mut Session, p: &Json, threaded: bool) -> Result { let sheet = target_sheet(s, p)?; let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3aceab0..adec9fa 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 33fdc11..ba096cf 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2795,3 +2795,19 @@ fn a_decrypted_workbook_never_overwrites_its_encrypted_original() { assert_eq!(std::fs::read(&path).unwrap(), bytes, "the encrypted original is untouched"); 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()); +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 5e859c6..82ce68a 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -338,6 +338,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 d0eef85..2e39f33 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2281,15 +2281,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)), } }