Merge pull request #780 from vangel76:fix/hyperlink-scheme-777

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) |

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),

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) {

crates/engine/src/cmd/edit.rs

@@ -503,7 +503,7 @@ fn select_col(s: &mut Session, _: &Json) -> Result<Json> {
     Ok(json!({"selection": d.selection.a1()}))
 }
 
-fn go_to(s: &mut Session, p: &Json) -> Result<Json> {
+pub(super) fn go_to(s: &mut Session, p: &Json) -> Result<Json> {
     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()?;

crates/engine/src/cmd/insert.rs

@@ -157,6 +157,15 @@ pub fn specs() -> Vec<CommandSpec> {
             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<Json> {
     })
 }
 
+/// 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<Json> {
+    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.

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;

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));
+        }
+    }
+}

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();

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",

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)),
     }
 }