Merge branch 'main' into fix/sec-protection-permissions

apps/gridcraft-cli/src/check/package.rs

@@ -6,10 +6,6 @@ const MAX_XML_PART: u64 = 64 * 1024 * 1024;
 /// Most zip entries looked at while validating a written package.
 const MAX_ENTRIES: usize = 200_000;
 
-/// Decompressed bytes that comparing one package's relationship kinds may read in total: the
-/// relationship parts themselves and the targets whose content is checked.
-const RELATIONSHIP_BUDGET: u64 = 512 * 1024 * 1024;
-
 /// Relationship kinds (lower case) whose absence after a save isn't a loss: the calculation chain is
 /// a cache Excel rebuilds, and GridCraft never writes it.
 pub(super) const REBUILT_KINDS: [&str; 1] = ["calcchain"];
@@ -121,30 +117,6 @@ fn read_zip_part(zip: &mut Zip<'_>, name: &str) -> Result<Vec<u8>, String> {
     Ok(buf)
 }
 
-enum PartError {
-    /// Reading the part would spend more than the remaining budget.
-    OverBudget,
-    Unreadable,
-}
-
-/// Reads a part like [`read_zip_part`], charging the decompressed bytes to `budget`.
-fn read_zip_part_within(zip: &mut Zip<'_>, name: &str, budget: &mut u64) -> Result<Vec<u8>, PartError> {
-    let f = zip.by_name(name).map_err(|_| PartError::Unreadable)?;
-    // One byte past what is left shows that the part doesn't fit.
-    let limit = MAX_XML_PART.min(budget.saturating_add(1));
-    let mut buf = Vec::new();
-    // Bytes read before a decompression error still count against the budget.
-    let res = f.take(limit).read_to_end(&mut buf);
-    let read = buf.len() as u64;
-    if read > *budget {
-        *budget = 0;
-        return Err(PartError::OverBudget);
-    }
-    *budget -= read;
-    res.map_err(|_| PartError::Unreadable)?;
-    Ok(buf)
-}
-
 /// Whether the producer flagged its cached results as stale with `<calcPr fullCalcOnLoad="1">`:
 /// libraries that don't calculate write it, and store 0 or nothing as every formula's value.
 pub(super) fn full_calc_on_load(bytes: &[u8]) -> bool {
@@ -260,30 +232,19 @@ pub fn validate_package(bytes: &[u8]) -> Result<Vec<String>, String> {
 /// nothing are left out, as a save may rightly drop them: a link to a place in the workbook
 /// (`#Sheet1!A1`, written as a cell's `location`) and a target part with nothing in it.
 pub(super) fn relationship_kinds(bytes: &[u8]) -> Result<BTreeMap<String, String>, String> {
-    relationship_kinds_within(bytes, RELATIONSHIP_BUDGET)
-}
-
-/// [`relationship_kinds`] reading at most `budget` decompressed bytes. `Err` once the relationship
-/// parts alone need more: their kinds can't be listed completely.
-fn relationship_kinds_within(bytes: &[u8], mut budget: u64) -> Result<BTreeMap<String, String>, String> {
     let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| format!("not a zip package: {e}"))?;
     let names = zip_names(&mut zip);
     let by_key: HashMap<String, &String> = names.iter().map(|n| (part_key(n), n)).collect();
     // Several relationships name the same part; each fresh check decompressed it again, so
     // repeated references to one highly compressible part multiplied the work. The verdict is
     // remembered per (part, named content) and the decompression the package can cause is
-    // budgeted as a whole, the relationship parts included: many large, unreferenced rels parts
-    // would otherwise each spend a full per-part allowance.
+    // budgeted as a whole (finding: repeated decompression).
     let mut verdicts: HashMap<(String, &'static str), bool> = HashMap::new();
+    let mut budget: u64 = 512 * 1024 * 1024;
     let mut kinds = BTreeMap::new();
     for n in names.iter().filter(|n| part_key(n).ends_with(".rels")) {
         let Some(src_dir) = rels_source_dir(&part_key(n)) else { continue };
-        let xml = match read_zip_part_within(&mut zip, n, &mut budget) {
-            Ok(xml) => xml,
-            Err(PartError::OverBudget) => return Err(format!("{n}: relationship check stopped at its decompression limit")),
-            Err(PartError::Unreadable) => continue,
-        };
-        let Ok(els) = xml_elements(&xml) else { continue };
+        let Ok(els) = read_zip_part(&mut zip, n).and_then(|xml| xml_elements(&xml)) else { continue };
         for (_, attrs) in els.iter().filter(|(el, _)| el == "Relationship") {
             let Some(kind) = attr(attrs, "Type").and_then(|t| t.rsplit('/').next()) else { continue };
             let target = attr(attrs, "Target").unwrap_or("");
@@ -368,36 +329,3 @@ fn holds_something_uncached(zip: &mut Zip<'_>, name: &str, marker: &'static str,
         buf.clear();
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use std::io::Write as _;
-
-    use super::*;
-
-    /// A package with `count` relationship parts of `size` bytes each, none of them describing an
-    /// existing part.
-    fn many_rels(count: usize, size: usize) -> Vec<u8> {
-        let mut z = zip::ZipWriter::new(Cursor::new(Vec::new()));
-        let opts = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated);
-        let head = r#"<?xml version="1.0"?><Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/custom" Target="https://example.com" TargetMode="External"/>"#;
-        let tail = "</Relationships>";
-        let filler = " ".repeat(size.saturating_sub(head.len() + tail.len()));
-        for i in 0..count {
-            z.start_file(format!("xl/_rels/part{i}.xml.rels"), opts).unwrap();
-            z.write_all(head.as_bytes()).unwrap();
-            z.write_all(filler.as_bytes()).unwrap();
-            z.write_all(tail.as_bytes()).unwrap();
-        }
-        z.finish().unwrap().into_inner()
-    }
-
-    #[test]
-    fn relationship_parts_count_against_the_decompression_budget() {
-        // Each part fits the per-part allowance, but together they need more than the budget.
-        let bytes = many_rels(8, 64 * 1024);
-        assert!(relationship_kinds_within(&bytes, 256 * 1024).is_err());
-        let kinds = relationship_kinds_within(&bytes, 1024 * 1024).unwrap();
-        assert_eq!(kinds.get("custom").map(String::as_str), Some("custom"));
-    }
-}

crates/engine/src/cmd.rs

@@ -503,10 +503,14 @@ pub(crate) fn refuse_unless_allowed(s: &Session, sheet: usize, what: &str, pick:
 pub(crate) fn locked_cell_error(sh: &gridcraft_model::Sheet, c: CellRef, unlocked: &[(String, String)]) -> Option<String> {
     let mut needs_password = None;
     for r in sh.edit_ranges.iter().filter(|r| r.contains(c)) {
-        if r.password.is_none() || unlocked.iter().any(|(s, t)| *s == sh.name && *t == r.title) {
+        // A range limited to Windows users or groups (a security descriptor) can't be checked
+        // here, so without a password it stays locked rather than opening to every editor.
+        if (r.password.is_none() && r.security_descriptor.is_none()) || unlocked.iter().any(|(s, t)| *s == sh.name && *t == r.title) {
             return None;
         }
-        needs_password.get_or_insert(r.title.as_str());
+        if r.password.is_some() {
+            needs_password.get_or_insert(r.title.as_str());
+        }
     }
     Some(match needs_password {
         Some(t) => format!("The range \"{t}\" needs its password before it can be changed (Review › Allow Edit Ranges)."),

crates/engine/src/cmd/data/filter.rs

@@ -270,6 +270,7 @@ pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) {
 
 pub(super) fn clear_filter(s: &mut Session, _: &Json) -> Result<Json> {
     let sheet = s.doc()?.wb.active_sheet();
+    crate::cmd::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?;
     edit(s, |cx| {
         let sh = cx.sheet_mut(sheet)?;
         if let Some(a) = sh.autofilter.as_mut() {
@@ -283,6 +284,7 @@ pub(super) fn clear_filter(s: &mut Session, _: &Json) -> Result<Json> {
 
 pub(super) fn reapply(s: &mut Session, _: &Json) -> Result<Json> {
     let sheet = s.doc()?.wb.active_sheet();
+    crate::cmd::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?;
     edit(s, |cx| {
         apply_filter(&mut cx.wb, sheet);
         cx.structural = true;

crates/engine/src/cmd/data/grouping.rs

@@ -21,6 +21,15 @@ fn outline_lines(s: &Session, p: &Json) -> Result<(bool, u32, u32)> {
     }
 }
 
+/// Outline commands change line visibility and grouping: a protected sheet must allow formatting that axis.
+fn refuse_unless_outline_allowed(s: &Session, sheet: usize, rows: bool) -> Result<()> {
+    if rows {
+        crate::cmd::refuse_unless_allowed(s, sheet, "Formatting rows", |p| p.format_rows)
+    } else {
+        crate::cmd::refuse_unless_allowed(s, sheet, "Formatting columns", |p| p.format_columns)
+    }
+}
+
 pub(super) fn group(s: &mut Session, p: &Json, on: bool) -> Result<Json> {
     let si = target_sheet(s, p)?;
     // The selection belongs to the active sheet, so another sheet needs its lines.
@@ -28,6 +37,7 @@ pub(super) fn group(s: &mut Session, p: &Json, on: bool) -> Result<Json> {
         return Err(bad(if on { "data.group" } else { "data.ungroup" }, "missing `rows` or `cols`: the selection is on the active sheet"));
     }
     let (rows, a, b) = outline_lines(s, p)?;
+    refuse_unless_outline_allowed(s, si, rows)?;
     edit(s, |cx| {
         let sh = cx.sheet_mut(si)?;
         let before = outline::summary_before(sh, rows);
@@ -58,6 +68,7 @@ pub(super) fn group(s: &mut Session, p: &Json, on: bool) -> Result<Json> {
 /// + button, saved to XLSX); showing keeps inner groups that were collapsed on their own collapsed.
 pub(super) fn detail(s: &mut Session, p: &Json, hide: bool) -> Result<Json> {
     let (rows, a, b) = outline_lines(s, p)?;
+    refuse_unless_outline_allowed(s, s.doc()?.wb.active_sheet(), rows)?;
     let want = p.get("level").and_then(Json::as_u64).map(|l| l.min(outline::MAX_LEVEL as u64) as u8);
     edit(s, |cx| {
         let si = cx.sheet_index();
@@ -123,6 +134,12 @@ pub(super) fn show_levels(s: &mut Session, p: &Json) -> Result<Json> {
             msg: "give `rows` or `cols`: the outline level to show (1 = top-level summaries only)".into(),
         });
     }
+    let active = s.doc()?.wb.active_sheet();
+    for (n, is_rows) in [(rows, true), (cols, false)] {
+        if n.is_some() {
+            refuse_unless_outline_allowed(s, active, is_rows)?;
+        }
+    }
     edit(s, |cx| {
         let si = cx.sheet_index();
         let sh = cx.sheet_mut(si)?;

crates/engine/src/cmd/format/sizes.rs

@@ -9,6 +9,17 @@ use super::{
 /// sheet is `sheet`, else the prefix, else the active one. The selection belongs to the active
 /// sheet, so another sheet needs explicit lines.
 fn line_targets(s: &Session, p: &Json, rows: bool) -> Result<(usize, Vec<(u32, u32)>)> {
+    let (sheet, lines) = resolve_lines(s, p, rows)?;
+    // Sizes and visibility are formatting of the line, which a protected sheet must allow.
+    if rows {
+        crate::cmd::refuse_unless_allowed(s, sheet, "Formatting rows", |p| p.format_rows)?;
+    } else {
+        crate::cmd::refuse_unless_allowed(s, sheet, "Formatting columns", |p| p.format_columns)?;
+    }
+    Ok((sheet, lines))
+}
+
+fn resolve_lines(s: &Session, p: &Json, rows: bool) -> Result<(usize, Vec<(u32, u32)>)> {
     let key = if rows { "rows" } else { "cols" };
     let mut sheet = target_sheet(s, p)?;
     if let Some(t) = str_param(p, key) {

crates/engine/src/cmd/insert.rs

@@ -1100,6 +1100,10 @@ fn insert_picture(s: &mut Session, p: &Json) -> Result<Json> {
         .unwrap_or_else(|| if data.starts_with(&[0x89, b'P', b'N', b'G']) { "image/png".into() } else { "image/jpeg".into() });
     let (w, h) = crate::io::image_size(&data).unwrap_or((320, 240));
     let d = s.doc()?;
+    // Sheet protection locks drawing objects, as for a text box's text.
+    if d.wb.active().is_some_and(|sh| sh.is_protected()) {
+        return Err(EngineError::Other(PROTECTED.into()));
+    }
     let id = d.wb.next_object_id();
     let at = cell_param(s, p, "at")?.unwrap_or(d.selection.active);
     let scale = (640.0 / w.max(1) as f32).min(1.0);

crates/engine/src/cmd/pivot/create.rs

@@ -67,9 +67,6 @@ fn field_list(v: Option<&Json>) -> Vec<Json> {
 }
 
 pub(super) fn insert_pivot(s: &mut Session, p: &Json) -> Result<Json> {
-    if s.doc()?.wb.protected_structure && matches!(str_param(p, "destination"), None | Some("new")) {
-        return Err(other("The workbook is protected and cannot be changed."));
-    }
     let source = source_param(s, p)?;
     let active = s.doc()?.wb.active_sheet();
     // Validate the source now (header names, size).
@@ -98,6 +95,10 @@ pub(super) fn insert_pivot(s: &mut Session, p: &Json) -> Result<Json> {
         let at = RangeRef::parse(body).map(|r| r.start).ok_or_else(|| bad("insert.pivotTable", format!("not a cell: `{dest}`")))?;
         Some((si, at))
     };
+    // Decided on the resolved destination: `New`, ` new ` and an empty one all make a sheet.
+    if target.is_none() && s.doc()?.wb.protected_structure {
+        return Err(other("The workbook is protected and cannot be changed."));
+    }
     let replace = bool_param(p, "replace").unwrap_or(false);
     let rows = field_list(p.get("rows"));
     let columns = field_list(p.get("columns"));

crates/engine/src/tests.rs

@@ -5572,3 +5572,62 @@ fn clearing_links_and_notes_keeps_allow_edit_ranges() {
     let sh = s.doc().unwrap().wb.active().unwrap().clone();
     assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty());
 }
+
+#[test]
+fn protected_sheet_refuses_layout_commands_its_permissions_do_not_allow() {
+    let mut s = s();
+    s.execute("range.setValues", json!({"range": "A1", "values": [["h"], [1], [2]]})).unwrap();
+    s.execute("data.filter", json!({"range": "A1:A3"})).unwrap();
+    s.execute("review.protectSheet", json!({})).unwrap();
+    for (id, params) in [
+        ("home.rowHeight", json!({"rows": "1:1", "height": 40})),
+        ("home.columnWidth", json!({"cols": "A:A", "width": 80})),
+        ("data.group", json!({"rows": "2:3"})),
+        ("data.showLevels", json!({"rows": 1})),
+        ("data.clearFilter", json!({})),
+        ("data.reapply", json!({})),
+        ("insert.picture", json!({"base64": "iVBORw0KGgo="})),
+    ] {
+        let err = s.execute(id, params).expect_err(id).to_string();
+        assert!(err.contains("protected"), "{id}: {err}");
+    }
+    // With the permissions ticked the same layout commands run.
+    s.execute("review.unprotectSheet", json!({})).unwrap();
+    s.execute("review.protectSheet", json!({"formatRows": true, "formatColumns": true, "autofilter": true})).unwrap();
+    for (id, params) in [
+        ("home.rowHeight", json!({"rows": "1:1", "height": 40})),
+        ("home.columnWidth", json!({"cols": "A:A", "width": 80})),
+        ("data.reapply", json!({})),
+    ] {
+        s.execute(id, params).unwrap_or_else(|e| panic!("{id}: {e}"));
+    }
+}
+
+#[test]
+fn an_edit_range_limited_to_windows_users_stays_locked() {
+    use gridcraft_model::EditRange;
+    let mut s = s();
+    let a1 = CellRef::parse("A1").unwrap();
+    let range = |sd: Option<&str>| EditRange {
+        title: "T".into(),
+        ranges: vec![gridcraft_core::RangeRef::cell(a1)],
+        password: None,
+        security_descriptor: sd.map(str::to_string),
+    };
+    let sh = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap();
+    sh.edit_ranges.push(range(None));
+    assert!(crate::cmd::locked_cell_error(sh, a1, &[]).is_none(), "an open range");
+    sh.edit_ranges = vec![range(Some("D:(A;;GA;;;WD)"))];
+    assert!(crate::cmd::locked_cell_error(sh, a1, &[]).is_some(), "a user-limited range");
+}
+
+#[test]
+fn pivot_new_sheet_destination_respects_workbook_structure_protection() {
+    let mut s = s();
+    s.execute("range.setValues", json!({"range": "A1", "values": [["Region", "Sales"], ["N", 1], ["S", 2]]})).unwrap();
+    s.execute("review.protectWorkbook", json!({"on": true})).unwrap();
+    for dest in ["new", "NEW", " new ", ""] {
+        assert!(s.execute("insert.pivotTable", json!({"source": "A1:B3", "destination": dest})).is_err(), "destination {dest:?}");
+    }
+    assert!(s.execute("insert.pivotTable", json!({"source": "A1:B3", "location": "New"})).is_err());
+}