From 2d889af2e25df86e6cc2e86f35559036f85e0b87 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 7 Oct 2026 22:11:44 -0400 Subject: [PATCH] M2: keep the system clipboard away from the control channel Commands run while handling a control request (engine.execute, ui.menu.invoke, ...) no longer read the system clipboard into Paste or write Copy/Cut text to it; only user-initiated runs do. Regression test included. append_story allocates note, endnote, editorial and table ids with saturating_add (no overflow panic on hostile ids), with a test. Also fixes a clippy nonminimal_bool in cmd/text.rs. Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/notes.rs | 2 +- crates/doc/src/slice.rs | 32 +++++++++++++++++++----- crates/engine/src/cmd/text.rs | 2 +- crates/ui-egui/src/control.rs | 46 +++++++++++++++++++++++++++++++++++ crates/ui-egui/src/lib.rs | 10 ++++++-- 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/crates/doc/src/notes.rs b/crates/doc/src/notes.rs index 0b0162d2c..701275d52 100644 --- a/crates/doc/src/notes.rs +++ b/crates/doc/src/notes.rs @@ -178,7 +178,7 @@ impl Story { } pub(crate) fn next_note_id(&self) -> u64 { - self.notes.iter().map(|n| n.id).max().unwrap_or(0) + 1 + self.notes.iter().map(|n| n.id).max().unwrap_or(0).saturating_add(1) } /// Insert empty footnotes for `n` new reference characters before which `k` references sit. diff --git a/crates/doc/src/slice.rs b/crates/doc/src/slice.rs index 1d7067e8c..c86b6f265 100644 --- a/crates/doc/src/slice.rs +++ b/crates/doc/src/slice.rs @@ -136,13 +136,15 @@ impl Story { } if self.text.is_empty() { let (id, frames, rev, link) = (self.id, std::mem::take(&mut self.frames), self.rev, self.link.take()); - *self = Story { id, frames, rev: rev + 1, link, ..other }; + *self = Story { id, frames, rev: rev.saturating_add(1), link, ..other }; return; } if !other.tables.is_empty() { - let first = self.tables.keys().max().copied().unwrap_or(0) + 1; + let mut next = self.tables.keys().max().copied().unwrap_or(0).saturating_add(1); let mut ids = std::collections::HashMap::new(); - for (new, (old, t)) in (first..).zip(std::mem::take(&mut other.tables)) { + for (old, t) in std::mem::take(&mut other.tables) { + let new = next; + next = next.saturating_add(1); let mut t = Arc::unwrap_or_clone(t); t.id = new; ids.insert(old, new); @@ -165,18 +167,18 @@ impl Story { self.notes.push(Arc::new(crate::notes::Footnote { id, text: n.text.clone() })); } for n in other.endnotes { - let id = self.endnotes.iter().map(|x| x.id).max().unwrap_or(0) + 1; + let id = self.endnotes.iter().map(|x| x.id).max().unwrap_or(0).saturating_add(1); self.endnotes.push(Arc::new(crate::notes::Footnote { id, text: n.text.clone() })); } for n in other.editorial { - let id = self.editorial.iter().map(|x| x.id).max().unwrap_or(0) + 1; + let id = self.editorial.iter().map(|x| x.id).max().unwrap_or(0).saturating_add(1); self.editorial.push(Arc::new(crate::endnotes::EditorialNote { id, ..(*n).clone() })); } self.anchors.extend(other.anchors); self.xrefs.extend(other.xrefs); self.index_refs.extend(other.index_refs); self.objects.extend(other.objects); - self.rev += 1; + self.rev = self.rev.saturating_add(1); } } @@ -211,4 +213,22 @@ mod tests { assert_eq!(part.notes.len(), 1); assert!(a.extract(2..8).notes.is_empty()); } + + #[test] + fn append_story_with_maximal_ids_does_not_overflow() { + let mut a = Story::with_text(StoryId(1), "Alpha", ParaFormat::default()); + a.tables.insert(u64::MAX, Arc::new(crate::table::Table::new(u64::MAX, 1, 1, 0, 0, 100.0))); + let note = |id| Arc::new(crate::notes::Footnote { id, text: Story::with_text(StoryId(0), "n", ParaFormat::default()) }); + a.endnotes.push(note(u64::MAX)); + a.notes.push(note(u64::MAX)); + a.rev = u64::MAX; + let mut b = Story::with_text(StoryId(2), "Beta", ParaFormat::default()); + b.tables.insert(1, Arc::new(crate::table::Table::new(1, 1, 1, 0, 0, 100.0))); + b.endnotes.push(note(1)); + b.notes.push(note(1)); + a.append_story(b); + assert_eq!(a.text, "Alpha\nBeta"); + assert_eq!(a.endnotes.len(), 2); + assert_eq!(a.notes.len(), 2); + } } diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 553e27721..d6c79d0c3 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -810,7 +810,7 @@ pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result { let doc = s.doc()?.uid; let mut f = typing_format(s).cloned().unwrap_or(TypingFormat { doc, at, attrs: CharAttrs::default(), cleared: vec![] }); f.attrs.merge(&a); - f.cleared.retain(|k| !attrs.get(k).is_some_and(|v| !v.is_null()) && !cleared.contains(k)); + f.cleared.retain(|k| attrs.get(k).is_none_or(|v| v.is_null()) && !cleared.contains(k)); f.cleared.extend(cleared); s.typing_format = Some(f); return ok(); diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index a8d386775..005b43868 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -99,7 +99,15 @@ fn key_from(name: &str) -> Option { }) } +/// Handle one control request. Commands it runs leave the user's system clipboard alone. pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome { + let was = std::mem::replace(&mut app.in_control, true); + let out = handle_request(app, ctx, req); + app.in_control = was; + out +} + +fn handle_request(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome { let p = &req.params; let s = |k: &str| p.get(k).and_then(Value::as_str); match req.method.as_str() { @@ -367,3 +375,41 @@ pub fn save_screenshot(app: &mut DesignApp, image: &egui::ColorImage, path: Opti None => json!({"ok": false, "error": "no writer configured"}), } } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }; + + #[test] + fn control_paste_never_reads_the_system_clipboard() { + let calls = Arc::new(AtomicUsize::new(0)); + let seen = calls.clone(); + let services = crate::Services { + clipboard_text: Some(Box::new(move || { + seen.fetch_add(1, Ordering::SeqCst); + Some("SECRET".into()) + })), + ..Default::default() + }; + let mut app = DesignApp::new(designcraft_engine::Session::new(), services); + app.run("file.new", json!({})).unwrap(); + let r = app.run("frame.create", json!({"rect": [72, 72, 300, 200], "content": "text", "text": "words"})).unwrap(); + let sid = r["story"].as_u64().unwrap(); + assert!(app.session.active().unwrap().selection.text.is_some(), "the new frame is being typed in"); + let text = |app: &DesignApp| app.session.active().unwrap().doc.story(designcraft_doc::StoryId(sid)).unwrap().text.clone(); + let ctx = egui::Context::default(); + let (req, _rx) = ControlRequest::new("engine.execute", json!({"command": "edit.paste"})); + let _ = handle(&mut app, &ctx, &req); + assert_eq!(calls.load(Ordering::SeqCst), 0, "the control channel must not read the system clipboard"); + assert!(!text(&app).contains("SECRET")); + assert!(!app.in_control); + // The same command chosen by the user does paste the system clipboard. + let _ = app.run("edit.paste", json!({})); + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert!(text(&app).contains("SECRET")); + } +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 47557a871..eb2e34975 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -423,6 +423,9 @@ pub struct DesignApp { pub last_recovery: f64, /// The egui context (set by the first frame): copied text goes to the system clipboard through it. egui_ctx: Option, + /// A control-channel request is being handled: commands it runs never read or write the + /// user's system clipboard (a control client must not see or replace it). + pub(crate) in_control: bool, } impl DesignApp { @@ -460,6 +463,7 @@ impl DesignApp { last_time: 0.0, last_recovery: 0.0, egui_ctx: None, + in_control: false, } } @@ -503,13 +507,15 @@ impl DesignApp { if let Some(r) = menus::run_ui(self, id, ¶ms) { return r; } - let params = self.with_system_clipboard(id, params); + // Only user-initiated runs touch the system clipboard, never the control channel's. + let system_clipboard = !self.in_control; + let params = if system_clipboard { self.with_system_clipboard(id, params) } else { params }; let r = self.session.execute(id, ¶ms).map_err(|e| e.to_string()); self.after_engine(); match &r { Err(e) => self.status(e.clone()), // Copied text goes to the system clipboard too, whichever way Copy was chosen. - Ok(v) if matches!(id, "edit.copy" | "edit.cut") => { + Ok(v) if system_clipboard && matches!(id, "edit.copy" | "edit.cut") => { if let (Some(t), Some(ctx)) = (v.get("text").and_then(Value::as_str), &self.egui_ctx) { ctx.copy_text(t.to_string()); ctx.request_repaint();