Merge pull request #777 from skylartaylor/document-link-schemes

Open only web links and local libraries from a document
This commit is contained in:
skylar taylor-barrick
2026-10-17 14:34:58 -06:00
committed by GitHub
21 changed files with 294 additions and 27 deletions
Generated
+6
View File
@@ -856,6 +856,11 @@ dependencies = [
"libc",
]
[[package]]
name = "craft-docpath"
version = "0.1.0"
source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09a67fb9a3fbd17ffe63#82a24b006eefa036b43c09a67fb9a3fbd17ffe63"
[[package]]
name = "crc32fast"
version = "1.5.2"
@@ -1511,6 +1516,7 @@ name = "figcraft-engine"
version = "0.6.0"
dependencies = [
"base64 0.22.1",
"craft-docpath",
"figcraft-collab",
"figcraft-color",
"figcraft-doc",
+3
View File
@@ -32,6 +32,9 @@ figcraft-mcp = { path = "crates/mcp" }
figcraft-collab-net = { path = "crates/collab-net" }
figcraft-plugin = { path = "crates/plugin" }
# Which paths stored in a document may be followed (craft-libs).
craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" }
# foundation
kurbo = { version = "0.13", features = ["serde"] }
thiserror = "2"
+1 -1
View File
@@ -182,7 +182,7 @@ impl Config {
return Err("`idle_days` must be from 0 to 3650".into());
}
let terms = self.terms_url.trim();
if !terms.is_empty() && !(terms.starts_with("https://") || terms.starts_with("http://")) || terms.contains(char::is_whitespace) {
if !(terms.is_empty() || terms.starts_with("https://") || terms.starts_with("http://")) || terms.contains(char::is_whitespace) {
return Err("`terms_url` must be a web address (https://…)".into());
}
if self.operator_contact.chars().count() > 120 || self.operator_contact.chars().any(char::is_control) || terms.len() > 500 {
+1 -1
View File
@@ -203,7 +203,7 @@ fn pairs(query: &str) -> Result<Vec<(String, String)>, String> {
fn with_port(authority: &str, default: u16) -> String {
let has_port = match authority.rsplit_once(':') {
// `[v6]` alone has colons inside the brackets only.
Some((h, p)) => !(h.starts_with('[') && !h.ends_with(']')) && !p.contains(']'),
Some((h, p)) => (!h.starts_with('[') || h.ends_with(']')) && !p.contains(']'),
None => false,
};
if has_port { authority.to_string() } else { format!("{authority}:{default}") }
+1
View File
@@ -20,6 +20,7 @@ figcraft-render = { workspace = true }
figcraft-format = { workspace = true }
figcraft-svg = { workspace = true }
figcraft-pdf = { workspace = true }
craft-docpath = { workspace = true }
figcraft-plugin = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
+1 -3
View File
@@ -691,9 +691,7 @@ fn push_changes(s: &mut Session, p: &Value) -> Result<Value> {
let groups: BTreeSet<String> = n
.overrides
.iter()
.filter(|g| {
!matches!(g.as_str(), "size" | "resizing" | "position" | "component" | "props" | SLOT) && !(d == inst && g.as_str() == "name")
})
.filter(|g| !(matches!(g.as_str(), "size" | "resizing" | "position" | "component" | "props" | SLOT) || d == inst && g.as_str() == "name"))
.cloned()
.collect();
for g in &groups {
+1 -1
View File
@@ -1356,7 +1356,7 @@ fn pattern_fill(s: &mut Session, p: &Value) -> Result<Value> {
const CMD: &str = "fill.pattern";
let src = req_id(CMD, p, "source")?;
let doc = s.doc()?;
if !doc.node(src).is_some_and(|n| !n.is_page()) {
if doc.node(src).is_none_or(|n| n.is_page()) {
return Err(bad(CMD, "`source` must be a layer"));
}
if req_targets(CMD, s, p)?.iter().any(|t| doc.is_ancestor(*t, src)) {
+1 -1
View File
@@ -1498,7 +1498,7 @@ fn patch_copies(doc: &mut Document, seeds: &[(NodeId, Vec<&'static str>)]) -> bo
let gs: Vec<&'static str> = groups
.iter()
.copied()
.filter(|g| !cn.overrides.contains(*g) && !(root && matches!(*g, "visible" | "name" | "position" | "exports" | "interactions")))
.filter(|g| !(cn.overrides.contains(*g) || root && matches!(*g, "visible" | "name" | "position" | "exports" | "interactions")))
.collect();
if gs.is_empty() {
continue;
+48
View File
@@ -0,0 +1,48 @@
//! Which URLs stored in a document may be handed to the browser or the system opener.
//!
//! A document can carry URLs that the app opens for whoever views it: a prototype's Open link
//! action ([`crate::proto_run`]) and Dev Mode's dev resources ([`crate::dev_resources`]). Both end
//! up at the platform's opener, so a document that someone else made chooses what that opener is
//! asked to do. Only the schemes a browser would follow on a plain link are allowed:
//!
//! - `javascript:` runs script. In the web build the opener is `window.open`, which executes it in
//! the app's own origin, so a saved document becomes stored XSS.
//! - `file:` and `smb:` reach the file system; on Windows a UNC host also sends credentials.
//! - Windows hands `ms-…:` schemes to application handlers, and macOS and Linux hand unknown
//! schemes to whichever application registered them.
//!
//! The check is lexical and does no I/O. It is deliberately an allow-list: a deny-list of known
//! dangerous schemes is outgrown by every new handler a platform adds.
/// The schemes a URL stored in a document may use.
const WEB_SCHEMES: [&str; 3] = ["http", "https", "mailto"];
/// Whether `url`, stored in a document, may be opened.
///
/// Requires a scheme from [`WEB_SCHEMES`], compared ignoring ASCII case. A URL with no scheme is
/// refused: the opener would resolve it against the app's own page on the web. Control characters
/// are refused anywhere, because browsers ignore some of them inside a scheme (`java\tscript:`
/// reaches a `javascript:` handler), and `http`/`https` must name a host.
pub fn openable(url: &str) -> bool {
let url = url.trim();
if url.is_empty() || url.chars().any(char::is_control) {
return false;
}
let Some((scheme, rest)) = url.split_once(':') else { return false };
// An RFC 3986 scheme, so a stray colon in text cannot look like one.
let mut chars = scheme.chars();
if !chars.next().is_some_and(|c| c.is_ascii_alphabetic()) || !chars.all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')) {
return false;
}
if !WEB_SCHEMES.iter().any(|s| scheme.eq_ignore_ascii_case(s)) {
return false;
}
if scheme.eq_ignore_ascii_case("mailto") {
// `mailto:` names an address.
return !rest.is_empty();
}
// `http` and `https` name a host, so the authority is required: `http:x` is not a web link,
// and `http:///p` has no host.
let Some(after) = rest.strip_prefix("//") else { return false };
!after.is_empty() && !after.starts_with('/')
}
+87
View File
@@ -0,0 +1,87 @@
//! A URL stored in a document reaches the browser or the system opener, so only the schemes a
//! browser follows on a plain link are allowed ([`crate::doc_urls`]).
use crate::doc_urls::openable;
#[test]
fn web_links_are_opened() {
for url in [
"http://example.com",
"https://example.com",
"https://example.com/a/b?c=d#e",
"HTTPS://EXAMPLE.COM",
"HtTp://example.com",
"mailto:a@b.c",
"mailto:a@b.c?subject=hi",
" https://example.com ",
"https://user:pw@example.com:8443/p",
] {
assert!(openable(url), "{url:?} should open");
}
}
#[test]
fn script_and_handler_schemes_are_refused() {
for url in [
// Runs in the app's own origin on the web build: a saved document would be stored XSS.
"javascript:alert(1)",
"JavaScript:alert(1)",
"jAvAsCrIpT:alert(document.cookie)",
"vbscript:msgbox(1)",
"data:text/html,<script>alert(1)</script>",
"blob:https://example.com/x",
// Reach the file system; a UNC host also sends Windows credentials.
"file:///etc/passwd",
"file://attacker.example/share/x",
"smb://attacker.example/share/x",
"webdav://attacker.example/x",
// Windows application handlers.
"ms-msdt:/id PCWDiagnostic",
"ms-officecmd:x",
"search-ms:query=x",
"shell:startup",
// Anything else a platform may have registered.
"tel:+1234",
"sms:+1234",
"ssh://host",
"ftp://host/x",
"figcraft://join?host=127.0.0.1",
"zoommtg://zoom.us/join?x",
"view-source:https://example.com",
"jar:https://example.com/x!/y",
] {
assert!(!openable(url), "{url:?} must be refused");
}
}
#[test]
fn a_scheme_is_required_and_control_characters_are_refused() {
for url in [
// No scheme: the web opener would resolve these against the app's own page.
"",
" ",
"example.com",
"/etc/passwd",
"//example.com",
"\\\\host\\share\\x",
"about",
// Browsers ignore some control characters inside a scheme, so a smuggled one must not
// leave `javascript` looking like an unknown scheme that we then refuse by luck.
"java\tscript:alert(1)",
"java\nscript:alert(1)",
"java\rscript:alert(1)",
"javascript\u{0}:alert(1)",
"https://example.com/\u{0}",
// A colon in ordinary text is not a scheme.
"see page 2: the diagram",
"1https://example.com",
// The allowed schemes still need their own part.
"mailto:",
"http:",
"http:example.com",
"http:///p",
"https://",
] {
assert!(!openable(url), "{url:?} must be refused");
}
}
+5
View File
@@ -26,6 +26,7 @@ pub mod dev_resources;
pub mod devcompare;
pub mod devinspect;
pub mod distance;
pub mod doc_urls;
pub mod drop;
pub mod file_access;
pub mod guard;
@@ -1576,6 +1577,8 @@ mod devfocus_tests;
#[cfg(test)]
mod devinspect_tests;
#[cfg(test)]
mod doc_urls_tests;
#[cfg(test)]
mod double_click_tests;
#[cfg(test)]
mod draw_tools_tests;
@@ -1636,6 +1639,8 @@ mod invite_links_tests;
#[cfg(test)]
mod keymap_tests;
#[cfg(test)]
mod library_path_tests;
#[cfg(test)]
mod library_range_styles_tests;
#[cfg(test)]
mod library_variables_tests;
+30
View File
@@ -0,0 +1,30 @@
//! A library link is stored in the document, so it is followed only on this computer
//! ([`crate::library_server`]): a path on another server would make opening someone else's file
//! contact that server, and on Windows send the credentials of whoever opened it.
use crate::Session;
#[test]
fn a_library_is_opened_only_from_this_computer() {
let mut s = Session::new();
for path in [
r"\\attacker.example\share\lib.figcraft",
"//attacker.example/share/lib.figcraft",
"file://attacker.example/share/lib.figcraft",
r"\\?\UNC\attacker.example\share\lib.figcraft",
r"\\.\pipe\lib",
"https://attacker.example/lib.figcraft",
] {
let e = s.fetch_library(path, false).expect_err(&format!("{path} must be refused"));
assert!(e.to_string().contains("only from this computer"), "{path}: {e}");
}
}
#[test]
fn a_local_library_path_is_still_followed() {
let mut s = Session::new();
// Refused for being absent, not for where it is: the gate let it through.
let missing = std::env::temp_dir().join("figcraft-no-such-library.figcraft");
let e = s.fetch_library(&missing.to_string_lossy(), false).expect_err("the file does not exist");
assert!(!e.to_string().contains("only from this computer"), "a local path must not be refused as remote: {e}");
}
+7
View File
@@ -129,6 +129,13 @@ impl Session {
/// document uses the library.
pub fn fetch_library(&mut self, path: &str, fresh: bool) -> Result<Arc<PublishedLibrary>> {
if !is_server_link(path) {
// A library link is stored in the document, so opening someone else's file would
// otherwise reach whatever path it names. Only a path on this computer is followed: a
// network share would make opening a document contact that server, and on Windows
// send the credentials of whoever opened it.
if !craft_docpath::is_local(path) {
return Err(EngineError::Other(format!("{path}: a library is opened only from this computer")));
}
return self.libraries.load(path);
}
let have = match self.libraries.cached(path) {
+8 -7
View File
@@ -1736,7 +1736,7 @@ impl<'d> Exporter<'d> {
let sp = &n.stroke;
let (w, h, weight) = (n.width, n.height, sp.weight);
let plain = n.strokes.iter().any(|p| p.visible) && sp.dashes.is_empty() && sp.sides.is_none() && sp.profile().is_none() && sp.brush.is_none();
if !plain || !(weight.is_finite() && weight > 0.0 && w.is_finite() && h.is_finite()) {
if !(plain && weight.is_finite() && weight > 0.0 && w.is_finite() && h.is_finite()) {
return;
}
let whole = matches!(n.kind, NodeKind::Ellipse { sweep, inner, .. } if inner == 0.0 && sweep.abs() >= std::f64::consts::TAU - 1e-9);
@@ -2217,12 +2217,13 @@ fn outlined(n: &Node) -> Option<(Node, String)> {
NodeKind::Line | NodeKind::Vector { .. } | NodeKind::Ellipse { .. } | NodeKind::Polygon { .. } | NodeKind::Star { .. } | NodeKind::Rectangle
);
let shown = |paints: &[Paint]| paints.iter().any(|p| p.visible);
if !shape
|| sp.brush.is_some()
|| !sp.profile().is_some_and(one_sided)
|| !(sp.weight.is_finite() && sp.weight > 0.0)
|| !shown(&n.strokes)
|| shown(&n.fills)
if !(shape
&& sp.brush.is_none()
&& sp.profile().is_some_and(one_sided)
&& sp.weight.is_finite()
&& sp.weight > 0.0
&& shown(&n.strokes)
&& !shown(&n.fills))
{
return None;
}
+1 -1
View File
@@ -2048,7 +2048,7 @@ impl<'a> Mapper<'a> {
self.warn("slot content assignment uses the empty GUID sentinel and was skipped");
continue;
}
if !self.nodes.get(&target).is_some_and(|n| n.get("isSlotContent").and_then(Kv::bool) == Some(true)) {
if self.nodes.get(&target).is_none_or(|n| n.get("isSlotContent").and_then(Kv::bool) != Some(true)) {
self.warn("slot content assignment target is missing or is not slot content");
continue;
}
+3 -2
View File
@@ -635,8 +635,9 @@ pub fn dev_resources_section(app: &mut FigApp, ui: &mut Ui, doc: &Document, id:
};
ui.painter().text(pos2(x, r.center().y + 7.0), Align2::LEFT_CENTER, widgets::elide(ui, &below, text_w), regular(10.0), t.text_secondary);
crate::a11y::link(&resp, name);
// Opened only on a click, never fetched.
if resp.on_hover_text(url.as_str()).clicked() {
// Opened only on a click, never fetched, and a dev resource's URL comes from the
// document, so only a web link opens.
if resp.on_hover_text(url.as_str()).clicked() && figcraft_engine::doc_urls::openable(url) {
ui.ctx().open_url(egui::OpenUrl::new_tab(url));
}
if let Some(i) = item.index {
+2
View File
@@ -173,6 +173,8 @@ mod plugins_browse_tests;
#[cfg(test)]
mod popup_width_tests;
#[cfg(test)]
mod present_links_tests;
#[cfg(test)]
mod present_tests;
#[cfg(test)]
mod present_view_tests;
+7 -5
View File
@@ -1047,12 +1047,14 @@ pub fn open_join(app: &mut FigApp, link: &str) {
app.ui.live.focus_join = true;
}
/// An invite link the app was opened with (`figcraft figcraft://join?…`): join at once; a link
/// that can't be used shows in the Join dialog with the reason.
/// An invite link the app was opened with (`figcraft figcraft://join?…`): the Join dialog opens
/// with the link filled in, so joining is the person's own choice.
///
/// The system hands the app any `figcraft://` link that is followed, which may be a page, a
/// message or a document rather than something the person meant to act on, so the link is shown
/// and confirmed rather than acted on.
pub fn open_invite(app: &mut FigApp, link: &str) {
if app.run("collab.join", json!({"link": link.trim()})).is_none() {
open_join(app, link.trim());
}
open_join(app, link.trim());
}
/// Escape closes the Join or Publish dialog (before the canvas's shortcuts see it: it doesn't
+7 -4
View File
@@ -47,12 +47,14 @@ fn escape_closes_the_join_dialog_and_keeps_the_selection() {
}
#[test]
fn opening_the_app_with_an_invite_link_joins() {
fn opening_the_app_with_an_invite_link_asks_before_joining() {
let (mut h, _) = Harness::new();
let link = "figcraft://join?host=127.0.0.1:47900&file=abc&token=AbCdEfGhIjKlMnOpQrStUv";
crate::live::open_invite(&mut h.app, &format!(" {link} "));
assert_eq!(h.app.session.live().status, Status::Connecting);
assert!(h.app.ui.live.join.is_none(), "no dialog: it joins at once");
// The system hands the app whatever `figcraft://` link was followed, which may come from a
// page or a message, so the link is shown and confirmed rather than acted on.
assert_eq!(h.app.ui.live.join.as_deref(), Some(link), "the dialog opens with the link filled in");
assert_eq!(h.app.session.live().status, Status::Off, "nothing is joined until the person confirms");
}
/// The browser's link: joins, can't host; joining fails here to show the dialog path.
@@ -82,7 +84,8 @@ fn a_link_that_cant_be_used_opens_the_dialog_with_it() {
h.app.session.live.install(Box::new(Browser));
crate::live::open_invite(&mut h.app, "figcraft://nope");
assert_eq!(h.app.ui.live.join.as_deref(), Some("figcraft://nope"), "the link stays in the field to fix");
assert!(h.app.ui.last_error.as_deref().is_some_and(|e| e.contains("not a live session invite")));
// The link is not tried until the person presses Join, so its reason shows then, not now.
assert!(h.app.ui.last_error.is_none());
assert_eq!(h.app.session.live().status, Status::Off);
}
+3 -1
View File
@@ -865,7 +865,9 @@ impl Presenter {
}
Action::CloseOverlay => self.close_overlay(now),
Action::OpenUrl { url } => {
if !url.trim().is_empty() {
// The URL comes from the document and this action fires from whatever trigger the
// prototype sets, including a delay, so no click is implied: only a web link opens.
if figcraft_engine::doc_urls::openable(url) {
self.urls.push(url.clone());
}
true
+71
View File
@@ -0,0 +1,71 @@
//! A prototype's Open link action carries a URL from the document and fires from whatever trigger
//! the prototype sets, including a delay, so the player hands the opener only a web link
//! ([`figcraft_engine::doc_urls`]). Without that, a saved document opens `javascript:` in the web
//! build's own origin (stored XSS) and `smb:`/`ms-…:` through the desktop's handlers.
use std::sync::{Arc, Mutex};
use figcraft_engine::Session;
use serde_json::{Value, json};
use crate::Services;
use crate::present_tests::{Ui as Harness, id};
/// Records every URL the player asks the platform to open.
#[derive(Default)]
struct Opened(Arc<Mutex<Vec<String>>>);
impl Services for Opened {
fn open_url(&mut self, url: &str) {
if let Ok(mut v) = self.0.lock() {
v.push(url.to_string());
}
}
}
fn ex(s: &mut Session, cmd: &str, p: Value) -> Value {
s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd} {p}: {e}"))
}
/// A presented frame whose layer opens `url` 100ms after the screen appears — no click anywhere.
fn opens_after_a_delay(url: &str) -> Vec<String> {
let mut s = Session::new();
let f = id(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 300, "height": 400, "name": "A"})));
let layer = id(&ex(&mut s, "shape.rectangle", json!({"x": 10, "y": 10, "width": 30, "height": 30, "parent": f})));
ex(
&mut s,
"proto.interaction.add",
json!({"id": layer, "trigger": {"type": "afterDelay", "ms": 100}, "action": {"type": "openUrl", "url": url}, "transition": {"type": "instant"}}),
);
let seen = Arc::new(Mutex::new(Vec::new()));
let mut u = Harness::new(s);
u.app.services = Box::new(Opened(seen.clone()));
u.run("proto.present", json!({"start": f}));
// Well past the delay.
u.frames(40, Vec::new);
seen.lock().map(|v| v.clone()).unwrap_or_default()
}
#[test]
fn a_prototype_opens_web_links() {
assert_eq!(opens_after_a_delay("https://example.com/a"), ["https://example.com/a"]);
assert_eq!(opens_after_a_delay("mailto:a@b.c"), ["mailto:a@b.c"]);
}
#[test]
fn a_prototype_cannot_open_a_script_or_handler_url() {
for url in [
// In the web build the opener is `window.open`, which runs this in the app's own origin.
"javascript:alert(document.cookie)",
"JavaScript:alert(1)",
"data:text/html,<script>alert(1)</script>",
// Reaches the file system; a UNC host also sends Windows credentials.
"file:///etc/passwd",
"smb://attacker.example/share/x",
// Handed to an application handler by the desktop.
"ms-msdt:/id PCWDiagnostic",
"figcraft://join?host=127.0.0.1:47900&file=f&token=t",
] {
assert!(opens_after_a_delay(url).is_empty(), "{url:?} must never reach the opener");
}
}