diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 6197dd5e2..009d77f2a 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -809,10 +809,9 @@ fn split(s: &mut Session, _: &Json) -> Result { fn new_window(s: &mut Session, _: &Json) -> Result { let d = s.doc()?.clone(); - let mut copy = crate::DocState::new((*d.wb).clone(), d.path.clone(), format!("{}:2", d.display_title())); + let mut copy = crate::DocState::new((*d.wb).clone(), d.path.clone(), format!("{}:2", d.display_title()), d.trust); copy.selection = d.selection.clone(); copy.scripts_trusted = d.scripts_trusted; - copy.trust = d.trust; let i = s.add_document(copy); Ok(json!({"index": i})) } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 7aee50921..b3e7309e2 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -34,7 +34,7 @@ fn new_workbook(s: &mut Session, p: &Json) -> Result { Some(name) => { let wb = crate::sample::build(name).ok_or_else(|| bad("file.new", format!("unknown sample `{name}`")))?; let title = crate::sample::title(name); - s.add_document(DocState::new(wb, None, title)) + s.add_document(DocState::new(wb, None, title, crate::Trust::Local)) } None => s.new_workbook(), }; @@ -83,12 +83,11 @@ fn open(s: &mut Session, p: &Json) -> Result { { s.close_document(0); } - let mut doc = DocState::new(wb, path, title); + // Someone else may have made the file: its active content asks before it runs. + let mut doc = DocState::new(wb, path, title, crate::Trust::FromFile); if crate::io::FileKind::from_path(&name).is_none() && gridcraft_xlsx::sniff(&bytes) == gridcraft_xlsx::Format::Xlsx { doc.content_kind = Some(crate::io::FileKind::Xlsx); } - // Someone else may have made the file: its active content asks before it runs. - doc.trust = crate::Trust::FromFile; let i = s.add_document(doc); // The decrypted-workbook notice is for the info bar, not a message box. let shown: Vec<&str> = warnings.iter().map(String::as_str).filter(|w| *w != crate::io::DECRYPTED_NOTICE).collect(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5c6e9f565..1fd2f267b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -134,12 +134,13 @@ pub struct Clipboard { pub trust: Trust, } -/// Whether a document's active content (macros) may run without asking the person first. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize)] +/// Whether a document's active content (macros) may run without asking the person first. There +/// is no default: whoever makes a document says where it came from ([`DocState::new`]), so a new +/// way of building one from bytes can't come out trusted by omission. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)] #[serde(rename_all = "camelCase")] pub enum Trust { /// Made in this session: a new workbook or a sample. - #[default] Local, /// Opened from a file, or holding content pasted from one: active content asks before it /// runs. @@ -259,7 +260,10 @@ fn trim_to_budget(history: &mut Vec, byte_budget: usize) { } impl DocState { - pub fn new(mut wb: Workbook, path: Option, title: String) -> DocState { + /// A document for `wb`, which came from where `trust` says: [`Trust::FromFile`] for anything + /// read from bytes (a file, a download, a recovery copy), [`Trust::Local`] only for a workbook + /// made in this session. + pub fn new(mut wb: Workbook, path: Option, title: String, trust: Trust) -> DocState { let mut calc = Calc::new(); calc.recalc_all(&mut wb); let scripts_trusted = !cmd::extra::has_scripts(&wb); @@ -280,7 +284,7 @@ impl DocState { unlocked_ranges: vec![], content_kind: None, scripts_trusted, - trust: Trust::Local, + trust, } } pub fn is_dirty(&self) -> bool { @@ -480,7 +484,7 @@ impl Session { let name = wb.next_sheet_name(); wb.sheets.push(Arc::new(gridcraft_model::Sheet::new(name))); } - self.add_document(DocState::new(wb, None, format!("Book{n}"))) + self.add_document(DocState::new(wb, None, format!("Book{n}"), Trust::Local)) } /// Journals a command, then trims the oldest entries past either cap. A recording in progress diff --git a/crates/engine/src/sample.rs b/crates/engine/src/sample.rs index 129fe2171..90029bac3 100644 --- a/crates/engine/src/sample.rs +++ b/crates/engine/src/sample.rs @@ -26,7 +26,7 @@ pub fn build(name: &str) -> Option { _ => return None, }; } - s.add_document(DocState::new(wb, None, title(name))); + s.add_document(DocState::new(wb, None, title(name), crate::Trust::Local)); let ok = match name { "budget" => budget(&mut s), "sales" => sales(&mut s), diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs index cab24426e..b445377b7 100644 --- a/crates/engine/src/tests/seams.rs +++ b/crates/engine/src/tests/seams.rs @@ -961,6 +961,22 @@ fn documents_know_where_they_came_from() { assert_eq!(trust(&s), Trust::FromFile); } +#[test] +fn whoever_makes_a_document_says_where_it_came_from() { + // No default trust: a document built from bytes outside `file.open` (a recovery copy, a + // template) is untrusted because its maker says so, not trusted because it forgot to. + let mut s = Session::new(); + let bytes = gridcraft_xlsx::write_xlsx(&gridcraft_model::Workbook::new()).unwrap(); + let (wb, _) = crate::io::open_bytes("recovered.xlsx", &bytes).unwrap(); + s.add_document(crate::DocState::new(wb, None, "Recovered".into(), Trust::FromFile)); + assert_eq!(trust(&s), Trust::FromFile); + assert_eq!(s.execute("document.inspect", json!({})).unwrap()["trust"], "fromFile"); + s.add_document(crate::DocState::new(gridcraft_model::Workbook::new(), None, "Made here".into(), Trust::Local)); + assert_eq!(trust(&s), Trust::Local); + s.new_workbook(); + assert_eq!(trust(&s), Trust::Local); +} + #[test] fn pasting_from_a_file_carries_its_trust() { let mut s = s(); // document 0: Local diff --git a/crates/engine/tests/it/dynamicmetadata.rs b/crates/engine/tests/it/dynamicmetadata.rs index e1edfc958..f88805476 100644 --- a/crates/engine/tests/it/dynamicmetadata.rs +++ b/crates/engine/tests/it/dynamicmetadata.rs @@ -26,7 +26,7 @@ fn opened() -> Session { let bytes = io::save_bytes(&wb, "dynamic.xlsx").unwrap(); let (back, _) = io::open_bytes("dynamic.xlsx", &bytes).unwrap(); let mut s = Session::new(); - s.add_document(DocState::new(back, None, "dynamic".into())); + s.add_document(DocState::new(back, None, "dynamic".into(), gridcraft_engine::Trust::FromFile)); s } diff --git a/crates/engine/tests/it/long_operator_chains.rs b/crates/engine/tests/it/long_operator_chains.rs index 97c95ce38..e5e5af4b7 100644 --- a/crates/engine/tests/it/long_operator_chains.rs +++ b/crates/engine/tests/it/long_operator_chains.rs @@ -78,7 +78,7 @@ fn longest_flat_chains_survive_editing_saving_and_reopening() { let bytes = io::save_bytes(wb, "chains.xlsx").expect("save"); let (reopened, _) = io::open_bytes("chains.xlsx", &bytes).expect("open"); let mut session = Session::new(); - session.add_document(DocState::new(reopened, None, "chains".into())); + session.add_document(DocState::new(reopened, None, "chains".into(), gridcraft_engine::Trust::FromFile)); session.execute("formulas.calculateNow", json!({})).expect("recalc"); assert_eq!(value(&session, "C2"), Value::Number(4096.0)); assert_eq!(value(&session, "C3"), Value::Number(-4094.0)); diff --git a/crates/ui-egui/tests/it/interaction.rs b/crates/ui-egui/tests/it/interaction.rs index 46e981d0a..08317aad3 100644 --- a/crates/ui-egui/tests/it/interaction.rs +++ b/crates/ui-egui/tests/it/interaction.rs @@ -84,7 +84,7 @@ fn horizontal_scrollbar_pages_and_drags_across_a_wide_sheet() { // Column A frozen: a page is the scrolling part only. sh.freeze = Some((0, 1)); let mut s = Session::new(); - s.add_document(gridcraft_engine::DocState::new(wb, None, "Book1".into())); + s.add_document(gridcraft_engine::DocState::new(wb, None, "Book1".into(), gridcraft_engine::Trust::Local)); let mut h = harness(s); let track = h.state().grid.hscroll.expect("the tab bar has a horizontal scrollbar"); assert!(track.width() >= 200.0, "the scrollbar gets a usable share of the tab bar: {track:?}"); diff --git a/crates/ui-egui/tests/it/scroll_position.rs b/crates/ui-egui/tests/it/scroll_position.rs index 7945eb344..bcf62b1ac 100644 --- a/crates/ui-egui/tests/it/scroll_position.rs +++ b/crates/ui-egui/tests/it/scroll_position.rs @@ -18,7 +18,7 @@ fn harness(freeze: Option<(u32, u32)>) -> Harness { sh.view_active = CellRef::parse("A31").unwrap(); sh.freeze = freeze; let mut session = Session::new(); - session.add_document(DocState::new(wb, None, "Book1".into())); + session.add_document(DocState::new(wb, None, "Book1".into(), gridcraft_engine::Trust::Local)); let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( |ui, app: &mut SheetApp| { app.logic(ui.ctx()); diff --git a/docs/architecture.md b/docs/architecture.md index 3fc6cb0b1..2a5bed44a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -71,13 +71,14 @@ Actions made of many commands (macros, a debugger that runs one step per call) u dialog (the dialog's confirming call is the step), and the buffer is independent of the journal (Automate › Record Actions still records through the journal). - **Document trust.** `DocState::trust` is `Local` for workbooks made in the session and - `FromFile` for workbooks opened from a file, and for workbooks that received a paste from one - that writes formulas or validation: Paste, Paste Special › All, All Except Borders, Formulas, - Formulas and Number Formats or Validation, and any linked paste other than column widths or - notes. Values, formats, column widths and notes carry nothing active. Active content in a - `FromFile` workbook asks before it runs. This - is provenance, which no decision changes; `DocState::scripts_trusted` is the separate - per-session consent for stored `automate.*` scripts (`automate.trustScripts`). + `FromFile` for workbooks opened from a file. `Trust` has no default: `DocState::new` takes it, + so every way of making a document says where its content came from. It is also `FromFile` for + workbooks that received a paste from one that writes formulas or validation: Paste, Paste + Special › All, All Except Borders, Formulas, Formulas and Number Formats or Validation, and any + linked paste other than column widths or notes. Values, formats, column widths and notes carry + nothing active. Active content in a `FromFile` workbook asks before it runs. This is provenance, + which no decision changes; `DocState::scripts_trusted` is the separate per-session consent for + stored `automate.*` scripts (`automate.trustScripts`). - **Workbook data in hidden names** (`cmd/stash.rs`). An item is split over hidden workbook-scope names `_gc__0`, `_gc__1`, … of string literals within Excel's 255- and 8,192-character limits, so it survives an xlsx round trip. Characters XML can't carry (controls