From 37419afe146f19134170887dd2b23b97e64f00ac Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 13:19:13 -0600 Subject: [PATCH 1/3] Open only web links and local libraries from a document A document carries things the app acts on for whoever opens it. Three of them were taken as given. Prototype Open link and Dev Mode dev resources both reached the platform's opener with any scheme. In the web build the opener is window.open, so a saved document could run script in the app's own origin; on the desktop any scheme went to the system's handler. Both now pass through the new engine module `doc_urls`, which allows http, https and mailto and nothing else. It is an allow-list on purpose: a list of known-bad schemes is outgrown by every handler a platform adds. A figcraft:// invite link joined the session at once. The system hands the app whatever such link was followed, which may come from a page or a message, so the Join dialog now opens with the link filled in and the person confirms. A library link stored in a document was loaded from wherever it pointed. Only a path on this computer is followed now, so opening someone else's file cannot make the app contact their server (which on Windows would also send credentials). Tests: each change has one that fails without it, verified by reverting. The prototype test fires through an afterDelay trigger, since no click is needed, and keeps a positive case so a refusal cannot pass vacuously. --- Cargo.lock | 6 ++ Cargo.toml | 3 + crates/engine/Cargo.toml | 1 + crates/engine/src/doc_urls.rs | 48 +++++++++++++ crates/engine/src/doc_urls_tests.rs | 87 +++++++++++++++++++++++ crates/engine/src/lib.rs | 5 ++ crates/engine/src/library_path_tests.rs | 30 ++++++++ crates/engine/src/library_server.rs | 7 ++ crates/ui-egui/src/dev_mode.rs | 5 +- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/live.rs | 12 ++-- crates/ui-egui/src/live_join_tests.rs | 11 +-- crates/ui-egui/src/present.rs | 4 +- crates/ui-egui/src/present_links_tests.rs | 71 ++++++++++++++++++ 14 files changed, 280 insertions(+), 12 deletions(-) create mode 100644 crates/engine/src/doc_urls.rs create mode 100644 crates/engine/src/doc_urls_tests.rs create mode 100644 crates/engine/src/library_path_tests.rs create mode 100644 crates/ui-egui/src/present_links_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 9baacd586..a61d87cc4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -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", diff --git a/Cargo.toml b/Cargo.toml index 8f125ac3e..fe0893282 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index f610c38b3..545d872cc 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -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 } diff --git a/crates/engine/src/doc_urls.rs b/crates/engine/src/doc_urls.rs new file mode 100644 index 000000000..3b8417cd2 --- /dev/null +++ b/crates/engine/src/doc_urls.rs @@ -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('/') +} diff --git a/crates/engine/src/doc_urls_tests.rs b/crates/engine/src/doc_urls_tests.rs new file mode 100644 index 000000000..d42029a0f --- /dev/null +++ b/crates/engine/src/doc_urls_tests.rs @@ -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,", + "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"); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 2b09c5c37..80de797eb 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -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; diff --git a/crates/engine/src/library_path_tests.rs b/crates/engine/src/library_path_tests.rs new file mode 100644 index 000000000..f15105dc8 --- /dev/null +++ b/crates/engine/src/library_path_tests.rs @@ -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}"); +} diff --git a/crates/engine/src/library_server.rs b/crates/engine/src/library_server.rs index dbd97a0b2..99d5f29af 100644 --- a/crates/engine/src/library_server.rs +++ b/crates/engine/src/library_server.rs @@ -129,6 +129,13 @@ impl Session { /// document uses the library. pub fn fetch_library(&mut self, path: &str, fresh: bool) -> Result> { 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) { diff --git a/crates/ui-egui/src/dev_mode.rs b/crates/ui-egui/src/dev_mode.rs index 0fedccd7f..f5c7e40ff 100644 --- a/crates/ui-egui/src/dev_mode.rs +++ b/crates/ui-egui/src/dev_mode.rs @@ -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 { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e6a14ede4..4dba320ca 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -171,6 +171,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; diff --git a/crates/ui-egui/src/live.rs b/crates/ui-egui/src/live.rs index 35af5d14a..513060aba 100644 --- a/crates/ui-egui/src/live.rs +++ b/crates/ui-egui/src/live.rs @@ -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 diff --git a/crates/ui-egui/src/live_join_tests.rs b/crates/ui-egui/src/live_join_tests.rs index d74521638..61567ce0d 100644 --- a/crates/ui-egui/src/live_join_tests.rs +++ b/crates/ui-egui/src/live_join_tests.rs @@ -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); } diff --git a/crates/ui-egui/src/present.rs b/crates/ui-egui/src/present.rs index 863b540fa..cc78c86bd 100644 --- a/crates/ui-egui/src/present.rs +++ b/crates/ui-egui/src/present.rs @@ -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 diff --git a/crates/ui-egui/src/present_links_tests.rs b/crates/ui-egui/src/present_links_tests.rs new file mode 100644 index 000000000..8223890c2 --- /dev/null +++ b/crates/ui-egui/src/present_links_tests.rs @@ -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>>); + +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 { + 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,", + // 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"); + } +} From f7188de93dfe820a1e434af58551b3eb23c1d97b Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 13:35:44 -0600 Subject: [PATCH 2/3] Drop a needless borrow in the dev-resource link gate clippy::needless_borrow; `url` is already a reference. Missed on review because a workspace clippy run aborts at the first failing crate, which was figcraft-format. --- crates/ui-egui/src/dev_mode.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dev_mode.rs b/crates/ui-egui/src/dev_mode.rs index f5c7e40ff..d34560885 100644 --- a/crates/ui-egui/src/dev_mode.rs +++ b/crates/ui-egui/src/dev_mode.rs @@ -637,7 +637,7 @@ pub fn dev_resources_section(app: &mut FigApp, ui: &mut Ui, doc: &Document, id: crate::a11y::link(&resp, name); // 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) { + 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 { From c7acd1a81d48f1bf49071a55877e012a395c5b6e Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 13:35:44 -0600 Subject: [PATCH 3/3] chore: clear the rustc 1.96 clippy lints that fail xtask ci Unrelated to the security changes in this branch; `main` already failed `clippy --workspace --all-targets -- -D warnings` without them. Eight clippy::nonminimal_bool sites across seven files, each taking clippy's own suggested rewrite. Every rewrite is pure boolean algebra with the negation kept outside, so the float comparisons and their NaN behaviour are untouched: !A && !B becomes !(A || B), and !opt.is_some_and(f) becomes opt.is_none_or(!f). Two sites needed explicit parentheses the suggestion omitted, where the rewritten term is the left operand of an && and || binds looser. designcraft cleared the same lint in bc840130; this is figcraft's equivalent. --- apps/figcraft-server/src/config.rs | 2 +- crates/collab-net/src/invite.rs | 2 +- crates/engine/src/cmd/component.rs | 4 +--- crates/engine/src/cmd/edit.rs | 2 +- crates/engine/src/components.rs | 2 +- crates/format/src/fig/export.rs | 15 ++++++++------- crates/format/src/fig/map.rs | 2 +- 7 files changed, 14 insertions(+), 15 deletions(-) diff --git a/apps/figcraft-server/src/config.rs b/apps/figcraft-server/src/config.rs index 101c7815f..d5f4f761e 100644 --- a/apps/figcraft-server/src/config.rs +++ b/apps/figcraft-server/src/config.rs @@ -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 { diff --git a/crates/collab-net/src/invite.rs b/crates/collab-net/src/invite.rs index bfa2e606e..617ad5187 100644 --- a/crates/collab-net/src/invite.rs +++ b/crates/collab-net/src/invite.rs @@ -203,7 +203,7 @@ fn pairs(query: &str) -> Result, 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}") } diff --git a/crates/engine/src/cmd/component.rs b/crates/engine/src/cmd/component.rs index a8901f241..ff7a56df4 100644 --- a/crates/engine/src/cmd/component.rs +++ b/crates/engine/src/cmd/component.rs @@ -691,9 +691,7 @@ fn push_changes(s: &mut Session, p: &Value) -> Result { let groups: BTreeSet = 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 { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 804c78198..a4769a35c 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1356,7 +1356,7 @@ fn pattern_fill(s: &mut Session, p: &Value) -> Result { 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)) { diff --git a/crates/engine/src/components.rs b/crates/engine/src/components.rs index ff582b8ad..56efc5c9a 100644 --- a/crates/engine/src/components.rs +++ b/crates/engine/src/components.rs @@ -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; diff --git a/crates/format/src/fig/export.rs b/crates/format/src/fig/export.rs index ca9e2682a..45234bd8d 100644 --- a/crates/format/src/fig/export.rs +++ b/crates/format/src/fig/export.rs @@ -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; } diff --git a/crates/format/src/fig/map.rs b/crates/format/src/fig/map.rs index 25540ff61..d9aba17ee 100644 --- a/crates/format/src/fig/map.rs +++ b/crates/format/src/fig/map.rs @@ -2045,7 +2045,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; }