diff --git a/ROADMAP.md b/ROADMAP.md --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-25 · **Change:** minor (AutoSum over a selected row puts its total to the right, and fills a selection's empty last row and column) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-25 · **Change:** minor (links to local files and folders open after a confirmation; AutoSum over a selected row puts its total to the right, and fills a selection's empty last row and column) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -135,6 +135,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-25 | Links to files and folders on this computer open: a hyperlink to a path or `file:` address, and a click on a `=HYPERLINK("path", …)` cell (which did nothing, for web links too), ask first and then open an existing document or folder with the system's program, relative paths from the workbook's folder; programs and scripts (`.exe`, `.sh`, `.app`…, also through a symlink or the executable bit), missing files and network shares stay refused (#2596) | | 2026-10-25 | AutoSum over a selection does what Excel does: a selected row gets one total to its right (in its last cell if that is empty), not a one-cell total under each cell; a block whose last column is empty gets each row's total there, one whose last row is empty each column's, and with both the corner gets the total of the block; otherwise totals go under each column as before | | 2026-10-24 | Go To lists the named ranges of every sheet (`Sheet2!Name` for a name scoped to another sheet; hidden names, constants and formulas left out) and the references entered before, a double-click goes to an entry, and the Reference box takes several names or references separated by commas (`Price, Type`, `B14:C22,F19:G30`) | | 2026-10-24 | Status bar: a right-click opens a menu that sets which selection statistics show (Average, Count and Sum by default; Numerical Count, Minimum and Maximum are new), kept with the preferences (`view.statusBar`) | diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -206,10 +206,22 @@ 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; + // http, https and mailto addresses and existing documents and folders (never programs). + let target = match gridcraft_engine::links::classify(target) { + gridcraft_engine::links::LinkKind::External(url) => url, + gridcraft_engine::links::LinkKind::Local(path) if std::path::Path::new(&path).is_absolute() => { + match gridcraft_engine::links::resolve_local(None, &path) { + Ok(full) => full.to_string_lossy().into_owned(), + Err(why) => { + log::warn!("refused to open {path:?}: {why}"); + return; + } + } + } + _ => { + log::warn!("refused to open {target:?}: only http, https, mailto and existing local files and folders are opened"); + return; + } }; let target = target.as_str(); let cmd = if cfg!(target_os = "macos") { diff --git a/crates/engine/src/cmd/insert/links.rs b/crates/engine/src/cmd/insert/links.rs --- a/crates/engine/src/cmd/insert/links.rs +++ b/crates/engine/src/cmd/insert/links.rs @@ -35,27 +35,36 @@ pub(super) 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. +/// Follows a link: a reference selects it, an `http`/`https`/`mailto` address or an existing file +/// or folder on this computer is handed to the host (`UiRequest::OpenUrl`); anything else is +/// refused ([`crate::links::classify_link`]). The cell's link is its hyperlink or the target of a +/// `=HYPERLINK()` formula. Opens no dialog; the interactive click asks for confirmation first, in +/// the UI. pub(super) 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(), + let (target, external) = match str_param(p, "url").or_else(|| str_param(p, "target")) { + Some(t) => (t.to_string(), false), 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 { + let Some(link) = crate::links::link_at(&s.doc()?.wb, sheet, at) else { return Err(EngineError::Other("There's no hyperlink in this cell.".into())); }; - h.target.clone() + link } }; - match crate::links::classify(&target) { + match crate::links::classify_link(&target, external) { crate::links::LinkKind::Internal(r) => crate::cmd::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::Local(path) => { + let base = s.doc()?.path.as_deref().and_then(|f| std::path::Path::new(f).parent().map(std::path::Path::to_path_buf)); + let full = crate::links::resolve_local(base.as_deref(), &path).map_err(EngineError::Other)?; + let opened = full.to_string_lossy().into_owned(); + s.ui_requests.push(crate::UiRequest::OpenUrl(opened.clone())); + Ok(json!({"opened": opened})) + } crate::links::LinkKind::Refused(why) => Err(EngineError::Other(why)), } } diff --git a/crates/engine/src/links.rs b/crates/engine/src/links.rs --- a/crates/engine/src/links.rs +++ b/crates/engine/src/links.rs @@ -1,9 +1,15 @@ //! 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. +//! somebody sent), so it is hostile input: only `http`, `https` and `mailto` addresses and files +//! or folders on this computer ever leave the workbook for the system; `smb:`, UNC paths, custom +//! URL handlers, `javascript:` and the like are refused. A local target is opened only after the +//! user confirms it, only when it exists, and never when it is a program or script +//! ([`resolve_local`]). Everything else without a scheme stays an in-workbook reference. -use gridcraft_core::RangeRef; +use std::path::{Path, PathBuf}; + +use gridcraft_core::{CellRef, RangeRef}; +use gridcraft_formula::Expr; +use gridcraft_model::Workbook; /// What a link target is. #[derive(Clone, Debug, PartialEq, Eq)] @@ -12,6 +18,9 @@ pub enum LinkKind { Internal(String), /// An address to hand to the system, trimmed: `http`, `https` or `mailto` only. External(String), + /// A file or folder on this computer (a `file:` address decoded, or a path), relative to the + /// workbook when it isn't absolute. Opened only after confirmation ([`resolve_local`]). + Local(String), /// Refused, with the reason to show. Refused(String), } @@ -24,12 +33,15 @@ pub fn classify(target: &str) -> LinkKind { if t.chars().any(char::is_control) { return LinkKind::Refused("The link contains control characters.".into()); } - if t.starts_with("\\\\") || t.starts_with("//") { + if is_unc_prefix(t) { return LinkKind::Refused("Links to network locations aren't opened.".into()); } if RangeRef::parse(t).is_some() { return LinkKind::Internal(t.to_string()); } + if let Some(path) = path_like(t) { + return local(path); + } 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, '+' | '-' | '.')); @@ -37,6 +49,12 @@ pub fn classify(target: &str) -> LinkKind { return LinkKind::Internal(t.to_string()); } let lower = scheme.to_ascii_lowercase(); + if lower == "file" { + return match file_url_path(rest) { + Ok(path) => local(&path), + Err(why) => LinkKind::Refused(why), + }; + } if ALLOWED.contains(&lower.as_str()) && !rest.trim().is_empty() { LinkKind::External(t.to_string()) } else { @@ -44,6 +62,271 @@ pub fn classify(target: &str) -> LinkKind { } } +/// Classifies the target of a stored hyperlink. `external` is the workbook's own mark that the +/// link leaves the workbook ("Existing File or Web Page"), which makes a bare file name such as +/// `notes.txt` a file instead of a defined name. +pub fn classify_link(target: &str, external: bool) -> LinkKind { + match classify(target) { + LinkKind::Internal(t) + if external && !t.trim().is_empty() && RangeRef::parse(&t).is_none() && !t.starts_with('#') && !(t.contains('!') && !t.contains('#')) => + { + local(t.split('#').next().unwrap_or_default()) + } + other => other, + } +} + +/// The text of a path that is plainly one: a drive (`C:\x`), an absolute, home or `.`/`..` +/// relative path, or anything with a separator that is not a `Sheet!A1` reference. Sheet and +/// defined names can't hold `/` or `\`, so those are never mistaken for references. +fn path_like(t: &str) -> Option<&str> { + let b = t.as_bytes(); + let drive = b.len() >= 3 && b[0].is_ascii_alphabetic() && b[1] == b':' && matches!(b[2], b'/' | b'\\'); + let rooted = + t.starts_with('/') || t.starts_with("~/") || t.starts_with("./") || t.starts_with("../") || t.starts_with(".\\") || t.starts_with("..\\"); + // `file:/x`, `smb:/h/s` and `javascript:a/b` carry a scheme: `classify` must see them. A single + // letter is a drive, not a scheme. + let has_scheme = t.split_once(':').is_some_and(|(s, _)| { + s.len() > 1 + && s.starts_with(|c: char| c.is_ascii_alphabetic()) + && s.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')) + }); + let separated = t.contains(['/', '\\']) && !t.contains('!') && !has_scheme; + (drive || rooted || separated).then(|| t.split('#').next().unwrap_or(t)) +} + +/// The path of a `file:` address: no host (or `localhost`), percent-decoded. Another host is a +/// network location and is refused. +fn file_url_path(rest: &str) -> Result { + let rest = rest.split(['#', '?']).next().unwrap_or_default(); + let path = match rest.strip_prefix("//") { + Some(after) => { + let (host, path) = after.split_at(after.find('/').unwrap_or(after.len())); + if !host.is_empty() && !host.eq_ignore_ascii_case("localhost") { + return Err("Links to network locations aren't opened.".into()); + } + path + } + None => rest, + }; + let decoded = percent_decode(path).ok_or_else(|| "The link isn't a valid file address.".to_string())?; + // `file:///C:/x` carries the drive after a slash. + let b = decoded.as_bytes(); + let decoded = if b.len() >= 3 && b[0] == b'/' && b[1].is_ascii_alphabetic() && b[2] == b':' { decoded[1..].to_string() } else { decoded }; + if decoded.is_empty() || decoded.chars().any(char::is_control) || is_unc_prefix(&decoded) { + return Err("The link isn't a valid file address.".into()); + } + Ok(decoded) +} + +fn percent_decode(s: &str) -> Option { + let b = s.as_bytes(); + let mut out = Vec::with_capacity(b.len()); + let mut i = 0; + while let Some(&c) = b.get(i) { + if c == b'%' { + let hex = s.get(i + 1..i + 3)?; + out.push(u8::from_str_radix(hex, 16).ok()?); + i += 3; + } else { + out.push(c); + i += 1; + } + } + String::from_utf8(out).ok() +} + +/// A network location on Windows: two leading separators of either kind (`//h/s`, `\\h\s`, `/\h/s`). +fn is_unc_prefix(path: &str) -> bool { + let b = path.as_bytes(); + b.len() >= 2 && matches!(b[0], b'/' | b'\\') && matches!(b[1], b'/' | b'\\') +} + +fn local(path: &str) -> LinkKind { + let path = path.trim(); + if path.is_empty() { + return LinkKind::Refused("The link has no file or folder.".into()); + } + if is_unc_prefix(path) { + return LinkKind::Refused("Links to network locations aren't opened.".into()); + } + if is_program(Path::new(path)) { + return LinkKind::Refused("Links to programs and scripts aren't opened. Open the file from your file manager if you trust it.".into()); + } + LinkKind::Local(path.to_string()) +} + +/// Extensions that run code when the system opens them. Compared without case and without the +/// trailing dots and spaces Windows ignores in a name. +const PROGRAMS: &[&str] = &[ + "exe", + "com", + "bat", + "cmd", + "msi", + "msp", + "scr", + "pif", + "ps1", + "psm1", + "vbs", + "vbe", + "vb", + "js", + "jse", + "wsf", + "wsh", + "hta", + "lnk", + "url", + "jar", + "jnlp", + "app", + "command", + "sh", + "bash", + "zsh", + "csh", + "ksh", + "desktop", + "run", + "bin", + "appimage", + "dll", + "ocx", + "drv", + "sys", + "vxd", + "wcx", + "cpl", + "msc", + "reg", + "inf", + "scf", + "chm", + "rdp", + "website", + "gadget", + "application", + "appref-ms", + "appcontent-ms", + "settingcontent-ms", + "appx", + "appxbundle", + "action", + "workflow", + "terminal", + "py", + "pyw", + "pl", + "rb", + "pkg", + "dmg", +]; + +fn is_program(path: &Path) -> bool { + let name = path.file_name().and_then(|n| n.to_str()).unwrap_or_default(); + let name = name.trim_end_matches(['.', ' ']); + let Some((_, ext)) = name.rsplit_once('.') else { return false }; + let ext = ext.trim().to_ascii_lowercase(); + PROGRAMS.contains(&ext.as_str()) +} + +/// The file or folder a [`LinkKind::Local`] names, ready for the system: relative paths are taken +/// from the workbook's folder (`base`, the folder of the saved file), `~/` from the home folder. +/// Refuses what doesn't exist, programs and scripts (by name, by what a link resolves to, and on +/// Unix by the executable bit), so a confirmed click can only open documents and folders. +pub fn resolve_local(base: Option<&Path>, target: &str) -> Result { + // Before anything touches the file system: resolving a share would contact it. + if is_unc_prefix(target) { + return Err("Links to network locations aren't opened.".into()); + } + let path = match target.strip_prefix("~/") { + Some(rest) => { + let home = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")).map(PathBuf::from); + home.ok_or_else(|| "The home folder isn't known.".to_string())?.join(rest) + } + None => PathBuf::from(target), + }; + let path = if path.is_absolute() || path.has_root() || target.as_bytes().get(1) == Some(&b':') { + path + } else { + let base = base.ok_or_else(|| "Save the workbook first: this link is relative to the workbook's folder.".to_string())?; + base.join(path) + }; + // A workbook kept on a share has relative links on that share. + if is_unc_prefix(&path.to_string_lossy()) { + return Err("Links to network locations aren't opened.".into()); + } + if is_program(&path) { + return Err("Links to programs and scripts aren't opened.".into()); + } + // Resolving follows links, and a link on this computer can lead to a share: look at where + // each one points before the file system is asked to follow it. + if leads_to_network(&path, 8) { + return Err("Links to network locations aren't opened.".into()); + } + let real = std::fs::canonicalize(&path).map_err(|_| format!("\"{}\" doesn't exist or can't be reached.", path.display()))?; + // A link on this computer can lead to a share (`\\?\UNC\…` once resolved on Windows). + let resolved = real.to_string_lossy(); + if resolved.starts_with("\\\\?\\UNC\\") || (is_unc_prefix(&resolved) && !resolved.starts_with("\\\\?\\")) { + return Err("Links to network locations aren't opened.".into()); + } + if is_program(&real) { + return Err("Links to programs and scripts aren't opened.".into()); + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let meta = std::fs::metadata(&real).map_err(|e| e.to_string())?; + if meta.is_file() && meta.permissions().mode() & 0o111 != 0 { + return Err("Links to programs and scripts aren't opened.".into()); + } + } + Ok(path) +} + +/// Whether `path`, or a folder on the way to it, is a symbolic link (or junction) to a network +/// location, looking at `read_link` only so nothing is contacted. Follows chains of links `depth` +/// deep; one that deep is refused as well. +fn leads_to_network(path: &Path, depth: u8) -> bool { + let mut seen = path.ancestors().collect::>(); + seen.reverse(); + seen.into_iter().any(|at| { + if !std::fs::symlink_metadata(at).is_ok_and(|m| m.file_type().is_symlink()) { + return false; + } + let Ok(to) = std::fs::read_link(at) else { return false }; + let text = to.to_string_lossy(); + if is_unc_prefix(&text) || text.starts_with("\\\\?\\UNC\\") { + return true; + } + let to = if to.is_absolute() { to } else { at.parent().map(|p| p.join(&to)).unwrap_or(to) }; + match depth.checked_sub(1) { + Some(left) => leads_to_network(&to, left), + None => true, + } + }) +} + +/// The target of the link in a cell: its stored hyperlink, or the first argument of a cell whose +/// formula is `=HYPERLINK(link, …)`, evaluated. `external` is the stored link's mark ([`classify_link`]). +pub fn link_at(wb: &Workbook, sheet: usize, at: CellRef) -> Option<(String, bool)> { + let sh = wb.sheet(sheet)?; + if let Some(h) = sh.hyperlinks.get(&at) { + return Some((h.target.clone(), h.external)); + } + let expr = sh.cell(at)?.formula.as_ref()?.expr()?; + let Expr::Call(name, args) = &expr else { return None }; + if name != "HYPERLINK" { + return None; + } + match gridcraft_calc::recalc::evaluate_expr(wb, sheet, at, args.first()?) { + gridcraft_core::Value::Text(t) if !t.trim().is_empty() => Some((t.to_string(), true)), + _ => None, + } +} + #[cfg(test)] mod tests { use super::*; @@ -57,7 +340,7 @@ mod tests { for bad in [ "javascript:alert(1)", "JaVaScRiPt:alert(1)", - "file:///etc/passwd", + "file://host/etc/passwd", "smb://host/share", "ms-excel:ofe|u|http://x", "\\\\host\\share\\x.exe", @@ -74,6 +357,110 @@ mod tests { } } + #[test] + fn local_files_and_folders_are_recognised() { + for (target, path) in [ + ("file:///home/u/a%20b.pdf", "/home/u/a b.pdf"), + ("FILE://localhost/tmp/x.txt", "/tmp/x.txt"), + ("file:///C:/Users/u/doc.txt", "C:/Users/u/doc.txt"), + ("file:///tmp/folder/", "/tmp/folder/"), + ("file:/tmp/a.txt", "/tmp/a.txt"), + ("/tmp/a.txt", "/tmp/a.txt"), + ("C:\\Docs\\a.pdf", "C:\\Docs\\a.pdf"), + ("docs/a.pdf", "docs/a.pdf"), + ("./a.pdf", "./a.pdf"), + ("..\\a.pdf", "..\\a.pdf"), + ("~/Documents", "~/Documents"), + ("Book.xlsx#Sheet1!A2", "Book.xlsx"), + ("/tmp/Book.xlsx#Sheet1!A2", "/tmp/Book.xlsx"), + ] { + let kind = classify(target); + if target == "Book.xlsx#Sheet1!A2" { + // A bare name is still told from a defined name only by the workbook's mark. + assert!(matches!(kind, LinkKind::Internal(_)), "{target}"); + assert_eq!(classify_link(target, true), LinkKind::Local(path.into()), "{target}"); + } else { + assert_eq!(kind, LinkKind::Local(path.into()), "{target}"); + } + } + assert_eq!(classify_link("notes.txt", true), LinkKind::Local("notes.txt".into())); + assert!(matches!(classify_link("notes.txt", false), LinkKind::Internal(_))); + assert!(matches!(classify_link("Total", false), LinkKind::Internal(_))); + assert!(matches!(classify_link("Sheet2!A1", true), LinkKind::Internal(_))); + assert!(matches!(classify_link("B2", true), LinkKind::Internal(_))); + } + + #[test] + fn local_programs_network_shares_and_bad_addresses_are_refused() { + for bad in [ + "file://host/share/x.txt", + "file:////host/share/x.txt", + "file:///tmp/run.sh", + "file:///tmp/run.SH", + "/usr/bin/tool.exe", + "C:\\x\\setup.msi", + "docs/macro.vbs", + "docs/app.exe . ", + "~/Desktop/Tool.app", + "file:///tmp/%00.txt", + "file:///tmp/%zz", + "file:///tmp/%ff.txt", + "file:", + "file:/%5Cserver/share/doc.txt", + "file:/%2Fserver/share/doc.txt", + "/\\server/share/doc.txt", + "\\/server/share/doc.txt", + "smb:/h/s", + "javascript:a/b", + "docs/setup.jnlp", + "docs/Snap.MSC", + "docs/tool.settingcontent-ms", + ] { + assert!(matches!(classify(bad), LinkKind::Refused(_)), "{bad:?} -> {:?}", classify(bad)); + } + assert!(matches!(classify_link("tool.exe", true), LinkKind::Refused(_))); + } + + #[test] + fn resolving_a_local_link_needs_an_existing_document_or_folder() { + #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("gridcraft-links-{}-{nonce}", std::process::id())); + std::fs::create_dir_all(dir.join("sub")).unwrap(); + std::fs::write(dir.join("doc.txt"), "x").unwrap(); + std::fs::write(dir.join("prog"), "#!/bin/sh\n").unwrap(); + // A relative link is taken from the workbook's folder; absolute ones and folders open as they are. + assert_eq!(resolve_local(Some(&dir), "doc.txt").unwrap(), dir.join("doc.txt")); + assert_eq!(resolve_local(Some(&dir), "sub").unwrap(), dir.join("sub")); + assert_eq!(resolve_local(None, dir.join("doc.txt").to_str().unwrap()).unwrap(), dir.join("doc.txt")); + assert!(resolve_local(Some(Path::new("//server/share")), "doc.txt").unwrap_err().contains("network")); + assert!(resolve_local(Some(Path::new("\\\\server\\share")), "doc.txt").unwrap_err().contains("network")); + assert!(resolve_local(None, "/\\server/share").unwrap_err().contains("network")); + assert!(resolve_local(Some(&dir), "\\\\server\\share").unwrap_err().contains("network")); + assert!(resolve_local(None, "doc.txt").unwrap_err().contains("Save the workbook")); + assert!(resolve_local(Some(&dir), "missing.txt").unwrap_err().contains("doesn't exist")); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + // Programs by the executable bit, and through a link with an innocent name. + std::fs::set_permissions(dir.join("prog"), std::fs::Permissions::from_mode(0o755)).unwrap(); + assert!(resolve_local(Some(&dir), "prog").is_err()); + std::os::unix::fs::symlink(dir.join("prog"), dir.join("looks-fine.txt")).unwrap(); + assert!(resolve_local(Some(&dir), "looks-fine.txt").is_err()); + // A link to a network location is refused without following it, also behind another link. + std::os::unix::fs::symlink("//server/share/x", dir.join("share.txt")).unwrap(); + std::os::unix::fs::symlink(dir.join("share.txt"), dir.join("via.txt")).unwrap(); + std::os::unix::fs::symlink("//server/share", dir.join("shared-dir")).unwrap(); + for name in ["share.txt", "via.txt", "shared-dir", "shared-dir/doc.txt"] { + assert!(resolve_local(Some(&dir), name).unwrap_err().contains("network"), "{name}"); + } + std::fs::write(dir.join("hidden.sh"), "x").unwrap(); + std::os::unix::fs::symlink(dir.join("hidden.sh"), dir.join("notes.txt")).unwrap(); + assert!(resolve_local(Some(&dir), "notes.txt").is_err()); + } + std::fs::remove_dir_all(dir).unwrap(); + } + #[test] fn references_stay_internal() { for r in ["Sheet2!A1", "'My Sheet'!B2:C3", "A1:B2", "#Sheet1!A1", "Total", "", " "] { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4430,11 +4430,11 @@ fn a_decrypted_workbook_never_overwrites_its_encrypted_original() { } #[test] -fn link_open_refuses_every_scheme_but_http_https_and_mailto() { +fn link_open_refuses_every_scheme_but_http_https_mailto_and_local_files() { let mut s = s(); - s.execute("insert.link", json!({"cell": "A1", "target": "file:///etc/passwd"})).unwrap(); + s.execute("insert.link", json!({"cell": "A1", "target": "file://host/etc/passwd"})).unwrap(); s.take_ui_requests(); - for url in ["file:///etc/passwd", "smb://h/s", "\\\\h\\s", "JAVASCRIPT:1"] { + for url in ["file://host/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()); @@ -4445,6 +4445,54 @@ fn link_open_refuses_every_scheme_but_http_https_and_mailto() { assert!(s.take_ui_requests().is_empty()); } +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn link_open_opens_existing_local_files_and_folders_but_not_programs() { + #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("gridcraft-locallink-{}-{nonce}", std::process::id())); + std::fs::create_dir_all(dir.join("reports")).unwrap(); + std::fs::write(dir.join("reports").join("q1.txt"), "x").unwrap(); + std::fs::write(dir.join("setup.exe"), "x").unwrap(); + let file = dir.join("reports").join("q1.txt"); + let mut s = s(); + // A hyperlink to a file, a `file:` address and a =HYPERLINK() formula all open it (#2596). + s.execute("cell.set", json!({"cell": "A1", "input": "q1"})).unwrap(); + s.execute("insert.link", json!({"cell": "A1", "target": file.to_str().unwrap()})).unwrap(); + s.execute( + "cell.set", + json!({"cell": "A2", "input": format!("=HYPERLINK(\"{}\",\"folder\")", dir.join("reports").to_str().unwrap().replace('"', "\"\""))}), + ) + .unwrap(); + s.execute("cell.set", json!({"cell": "A3", "input": format!("=HYPERLINK(\"{}\")", "setup.exe")})).unwrap(); + s.take_ui_requests(); + s.execute("link.open", json!({"cell": "A1"})).unwrap(); + assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl(file.to_string_lossy().into_owned())]); + s.execute("link.open", json!({"cell": "A2"})).unwrap(); + assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl(dir.join("reports").to_string_lossy().into_owned())]); + let url = format!("file://{}", file.to_str().unwrap()); + if cfg!(unix) { + s.execute("link.open", json!({"url": url})).unwrap(); + assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl(file.to_string_lossy().into_owned())]); + } + // Programs, missing files and network shares are still refused. + assert!(s.execute("link.open", json!({"cell": "A3"})).is_err()); + assert!(s.execute("link.open", json!({"url": dir.join("setup.exe").to_str().unwrap()})).is_err()); + assert!(s.execute("link.open", json!({"url": dir.join("missing.txt").to_str().unwrap()})).is_err()); + assert!(s.execute("link.open", json!({"url": "file://host/share/a.txt"})).is_err()); + assert!(s.take_ui_requests().is_empty()); + // A relative link is taken from the saved workbook's folder. + let book = dir.join("Book.xlsx"); + s.execute("cell.set", json!({"cell": "B1", "input": "rel"})).unwrap(); + s.execute("insert.link", json!({"cell": "B1", "target": "reports/q1.txt"})).unwrap(); + assert!(s.execute("link.open", json!({"cell": "B1"})).is_err(), "unsaved: no folder to take it from"); + s.execute("file.saveAs", json!({"path": book.to_str().unwrap()})).unwrap(); + s.take_ui_requests(); + s.execute("link.open", json!({"cell": "B1"})).unwrap(); + assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl(dir.join("reports/q1.txt").to_string_lossy().into_owned())]); + std::fs::remove_dir_all(dir).unwrap(); +} + #[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 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -481,19 +481,17 @@ 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. + // Asks before a web, e-mail or local file link leaves the workbook; OK runs `link.open`, + // which refuses anything else (and programs and scripts). "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}), - ) + let local = params.get("local").and_then(Json::as_bool).unwrap_or(false); + let ask = if local { + "This workbook asks to open the file or folder below with the program your computer uses for it. Files can contain harmful content: open it only if you trust the workbook." + } else { + "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." + }; + Dialog::form("openLink", "Open Hyperlink", "link.open", vec![Note(format!("{ask}\n\n{url}"))], json!({"url": url})) } "insertLink" => Dialog::form( "insertLink", diff --git a/crates/ui-egui/src/grid/interact.rs b/crates/ui-egui/src/grid/interact.rs --- a/crates/ui-egui/src/grid/interact.rs +++ b/crates/ui-egui/src/grid/interact.rs @@ -371,12 +371,12 @@ pub(super) fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Respo } } else if mods.command { // Ctrl+click (without Alt) on a hyperlink's text follows it instead of adding the cell to the selection. - if let Some(h) = sh.hyperlinks.get(&c).cloned() + if let Some((target, external)) = gridcraft_engine::links::link_at(wb, wb.active_sheet(), c) && primary_clicked && !mods.alt && p.x < geo.cell_rect(sh, c).left() + geo.cell_rect(sh, c).width() * 0.8 { - follow_link(app, &h.target); + follow_link(app, &target, external); } else { select_ranges(app, RangeRef::cell(c), true, c); } @@ -386,13 +386,13 @@ pub(super) fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Respo let _ = app.run("cell.toggleCheckbox", json!({"cell": c.a1()})); } // A click on a hyperlink's text follows it (like Excel); elsewhere in the cell selects. - if let Some(h) = sh.hyperlinks.get(&c).cloned() + if let Some((target, external)) = gridcraft_engine::links::link_at(wb, wb.active_sheet(), c) && primary_clicked && !mods.any() { let r = geo.cell_rect(sh, c); if p.x < r.left() + r.width() * 0.8 { - follow_link(app, &h.target); + follow_link(app, &target, external); } } } @@ -859,17 +859,26 @@ pub(super) 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) { - use gridcraft_engine::links::{LinkKind, classify}; - match classify(target) { +/// A click on a hyperlink (`external`: the workbook marks it as leaving the workbook). References +/// select; web and e-mail addresses and existing files and folders 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_link`]), the same one `link.open` applies. +pub fn follow_link(app: &mut SheetApp, target: &str, external: bool) { + use gridcraft_engine::links::{LinkKind, classify_link, resolve_local}; + match classify_link(target, external) { LinkKind::Internal(r) => { let _ = app.session.run("edit.goTo", json!({"reference": r.trim_start_matches('#')})); app.grid.ensure_visible = true; } LinkKind::External(url) => app.open_dialog("openLink", json!({"url": url})), + LinkKind::Local(path) => { + let file = app.session.active().and_then(|d| d.path.clone()); + let base = file.as_deref().and_then(|f| std::path::Path::new(f).parent()); + match resolve_local(base, &path) { + Ok(full) => app.open_dialog("openLink", json!({"url": full.to_string_lossy(), "local": true})), + Err(why) => app.message = Some(("Hyperlink".into(), why)), + } + } LinkKind::Refused(why) => app.message = Some(("Hyperlink".into(), why)), } } diff --git a/crates/ui-egui/src/grid/show.rs b/crates/ui-egui/src/grid/show.rs --- a/crates/ui-egui/src/grid/show.rs +++ b/crates/ui-egui/src/grid/show.rs @@ -630,10 +630,15 @@ pub(crate) fn paint_overlays( ); } // Hyperlink tooltip. - if let Some(h) = hover.and_then(|c| sh.hyperlinks.get(&c)) + if let Some(c) = hover && app.editor.is_none() + && let Some((target, _)) = gridcraft_engine::links::link_at(wb, si, c) { - let tip = h.tooltip.clone().unwrap_or_else(|| format!("{}\nClick once to follow. Click and hold to select this cell.", h.target)); + let tip = sh + .hyperlinks + .get(&c) + .and_then(|h| h.tooltip.clone()) + .unwrap_or_else(|| format!("{target}\nClick once to follow. Click and hold to select this cell.")); egui::Tooltip::always_open(ui.ctx().clone(), ui.layer_id(), egui::Id::unique("link_tip"), egui::PopupAnchor::Pointer).show(|ui| { ui.label(tip); }); diff --git a/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs b/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs --- a/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs +++ b/crates/ui-egui/tests/it/hyperlink_ctrl_click.rs @@ -8,9 +8,20 @@ use serde_json::json; type Harness = egui_kittest::Harness<'static, SheetApp>; +fn local_file() -> std::path::PathBuf { + let path = std::env::temp_dir().join(format!("gridcraft-ui-link-{}.txt", std::process::id())); + std::fs::write(&path, "x").unwrap(); + path +} + fn harness() -> Harness { let mut session = Session::new(); session.new_workbook(); + session.execute("cell.set", json!({"cell": "B6", "input": "file"})).unwrap(); + session.execute("insert.link", json!({"cell": "B6", "target": local_file().to_str().unwrap()})).unwrap(); + session.execute("cell.set", json!({"cell": "B8", "input": "=HYPERLINK(\"https://example.org/h\",\"go\")"})).unwrap(); + session.execute("cell.set", json!({"cell": "B10", "input": "tool"})).unwrap(); + session.execute("insert.link", json!({"cell": "B10", "target": "/tmp/setup.exe"})).unwrap(); session.execute("cell.set", json!({"cell": "B2", "input": "web"})).unwrap(); session.execute("insert.link", json!({"cell": "B2", "target": "https://example.org/a"})).unwrap(); session.execute("cell.set", json!({"cell": "B4", "input": "jump"})).unwrap(); @@ -95,3 +106,28 @@ fn ctrl_alt_click_on_the_link_text_still_extends_the_selection() { assert_eq!(d.selection.active.a1(), "B4", "the link was not followed"); assert!(h.state().dialog.is_none(), "no link dialog"); } + +#[test] +fn a_click_on_a_local_file_link_asks_before_opening_it() { + let mut h = harness(); + click(&mut h, "B6", Modifiers::NONE); + let d = h.state().dialog.as_ref().expect("a link to a file on this computer asks first (#2596)"); + assert_eq!(d.name, "openLink"); + assert!(h.state().message.is_none()); +} + +#[test] +fn a_click_on_a_hyperlink_formula_follows_it() { + let mut h = harness(); + click(&mut h, "B8", Modifiers::NONE); + let d = h.state().dialog.as_ref().expect("=HYPERLINK() cells follow their link on click (#2596)"); + assert_eq!(d.name, "openLink"); +} + +#[test] +fn a_click_on_a_link_to_a_program_is_refused_with_a_message() { + let mut h = harness(); + click(&mut h, "B10", Modifiers::NONE); + assert!(h.state().dialog.is_none()); + assert!(h.state().message.as_ref().is_some_and(|(_, why)| why.contains("programs and scripts")), "{:?}", h.state().message); +}