shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #111 from nihilist-narwhal/fix-protection-commands 

Protection: enforce locked cells for every command, keep hidden formulas hidden
This commit is contained in:
Brandon Thomas 2026-10-10 11:57:08 -04:00
parent a678c5d25f e294345599
commit 1cdee3ee02
7 changed files with 249 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
crates/engine/src/cmd/edit.rs
View File

@ -1175,7 +1175,11 @@ fn find(s: &mut Session, p: &Json) -> Result<Json> {
.cells
.iter()
.filter_map(|(c, cell)| {
let text = if values || cell.formula.is_none() { crate::display::cell_text(&d.wb, sh, c) } else { cell.input_text() };
let text = if values || cell.formula.is_none() || crate::display::formula_hidden(&d.wb, sh, c) {
crate::display::cell_text(&d.wb, sh, c)
} else {
cell.input_text()
};
matches(&text, &what, case, whole).then_some((c, text))
})
.collect();

47
crates/engine/src/cmd/mod.rs
View File

@ -209,6 +209,9 @@ pub struct Ctx<'a> {
pub sel: &'a mut Selection,
/// Rows whose automatic height should be recomputed (sheet, row).
pub fit_rows: Vec<(usize, u32)>,
/// The command applied sheet protection itself (row/column inserts and deletes the
/// protection allows), so cells it shifts aren't refused as edits of locked cells.
pub protection_checked: bool,
}
pub(crate) fn edit<R>(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result<R>) -> Result<R> {
@ -220,9 +223,12 @@ pub(crate) fn commit<R>(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result<R>)
let prof = std::env::var_os("GRIDCRAFT_PROFILE").is_some();
let t0 = std::time::Instant::now();
let mut sel = d.selection.clone();
let mut ctx = Ctx { wb: (*d.wb).clone(), changed: Vec::new(), structural: false, sel: &mut sel, fit_rows: Vec::new() };
let mut ctx = Ctx { wb: (*d.wb).clone(), changed: Vec::new(), structural: false, sel: &mut sel, fit_rows: Vec::new(), protection_checked: false };
let r = f(&mut ctx)?;
let Ctx { mut wb, changed, structural, mut fit_rows, .. } = ctx;
let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, .. } = ctx;
if !protection_checked {
check_protection(&d.wb, &wb)?;
}
if changed.len() <= 200_000 {
fit_rows.extend(changed.iter().map(|(s, c)| (*s, c.row)));
}
@ -250,6 +256,43 @@ pub(crate) fn commit<R>(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result<R>)
Ok(r)
}
/// Excel's message for an edit of a locked cell on a protected sheet.
pub(crate) const PROTECTED: &str = "The cell or chart you're trying to change is on a protected sheet.";
/// Refuses an edit that changed what a locked cell on a protected sheet holds (its value or
/// formula), whichever command made it. Sheets are matched by name, so moving, renaming or
/// deleting a sheet isn't an edit of its cells.
fn check_protection(old: &Workbook, new: &Workbook) -> Result<()> {
for sh in &old.sheets {
if !sh.is_protected() {
continue;
}
let Some(after) = new.sheet_index(&sh.name).and_then(|i| new.sheets.get(i)) else { continue };
if std::sync::Arc::ptr_eq(sh, after) || !after.is_protected() {
continue;
}
for c in sh.cells.diff(&after.cells) {
if !same_content(sh.cell(c), after.cell(c)) && old.styles.get(sh.style_id(c)).protection.locked {
return Err(EngineError::Other(PROTECTED.into()));
}
}
}
Ok(())
}
/// Two cells hold the same value or formula (formats aside).
fn same_content(a: Option<&gridcraft_model::Cell>, b: Option<&gridcraft_model::Cell>) -> bool {
let empty = |c: Option<&gridcraft_model::Cell>| c.is_none_or(|c| c.formula.is_none() && c.value.is_empty());
match (a, b) {
(Some(a), Some(b)) => match (&a.formula, &b.formula) {
(Some(x), Some(y)) => x.text == y.text && x.array == y.array,
(None, None) => a.value == b.value,
_ => false,
},
_ => empty(a) && empty(b),
}
}
/// Recomputes an automatic row height from its content: the largest font, and wrapped or
/// multi-line text. Rows with a user-set height keep it.
pub fn auto_row_height(wb: &mut Workbook, si: usize, row: u32) {

37
crates/engine/src/cmd/sheet.rs
View File

@ -195,6 +195,7 @@ fn cancel_copy_mode(s: &mut Session) {
fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
let (at, count) = lines(s, p, axis)?;
check_lines_protection(s, p, axis, at, count, true)?;
let sheet = target_sheet(s, p)?;
// Refuse to push data off the sheet.
let limit = if axis == Axis::Rows { MAX_ROWS } else { MAX_COLS };
@ -206,6 +207,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
}
let out = edit(s, |cx| {
let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default();
cx.protection_checked = true;
{
let sh = cx.sheet_mut(sheet)?;
match axis {
@ -234,9 +236,11 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
fn delete_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
let (at, count) = lines(s, p, axis)?;
check_lines_protection(s, p, axis, at, count, false)?;
let sheet = target_sheet(s, p)?;
let out = edit(s, |cx| {
let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default();
cx.protection_checked = true;
{
let sh = cx.sheet_mut(sheet)?;
let block = match axis {
@ -354,6 +358,39 @@ fn edit_doc<R>(d: &mut crate::DocState, f: impl FnOnce(&mut Ctx) -> Result<R>) -
super::commit(d, f)
}
/// Sheet protection for inserting or deleting rows/columns: the protection must allow it, and
/// deleted lines must not hold locked cells.
fn check_lines_protection(s: &Session, p: &Json, axis: Axis, at: u32, count: u32, insert: bool) -> Result<()> {
let sheet = target_sheet(s, p)?;
let wb = &s.doc()?.wb;
let Some(sh) = wb.sheet(sheet) else { return Ok(()) };
let Some(pr) = &sh.protection else { return Ok(()) };
let allowed = match (axis, insert) {
(Axis::Rows, true) => pr.insert_rows,
(Axis::Rows, false) => pr.delete_rows,
(Axis::Cols, true) => pr.insert_columns,
(Axis::Cols, false) => pr.delete_columns,
};
let locked = |st: gridcraft_model::StyleId| wb.styles.get(st).protection.locked;
let refuse = || EngineError::Other(PROTECTED.into());
if !allowed {
return Err(refuse());
}
if insert {
return Ok(());
}
let end = at.saturating_add(count - 1).min(if axis == Axis::Rows { MAX_ROWS - 1 } else { MAX_COLS - 1 });
let infos = if axis == Axis::Rows { &sh.rows } else { &sh.cols };
if (at..=end).any(|i| locked(infos.get(&i).and_then(|x| x.style).unwrap_or_default())) {
return Err(refuse());
}
let block = if axis == Axis::Rows { RangeRef::rows(at, end) } else { RangeRef::cols(at, end) };
if sh.cells.iter_range(block).any(|(_, cell)| locked(cell.style)) {
return Err(refuse());
}
Ok(())
}
fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
let i = target_sheet(s, p)?;
let d = s.doc()?;

17
crates/engine/src/display.rs
View File

@ -31,11 +31,28 @@ pub fn format(v: &Value, code: &str, wb: &Workbook) -> Formatted {
format_value(v, &parsed(code), wb.date_system)
}
/// Whether `c` has a formula that must not be shown: its cell is formatted Hidden on a
/// protected sheet. A spilled cell follows its anchor.
pub fn formula_hidden(wb: &Workbook, sheet: &Sheet, c: CellRef) -> bool {
if !sheet.is_protected() {
return false;
}
let at = match sheet.cell(c) {
Some(x) if x.formula.is_some() => c,
_ => match sheet.spill_ranges.iter().find(|(_, r)| r.contains(c)) {
Some((anchor, _)) => *anchor,
None => return false,
},
};
wb.styles.get(sheet.style_id(at)).protection.hidden
}
/// The text a cell shows (full precision General; the grid narrows General to fit the column).
pub fn cell_text(wb: &Workbook, sheet: &Sheet, c: CellRef) -> String {
let v = sheet.value(c);
if sheet.show_formulas
&& let Some(f) = sheet.cell(c).and_then(|x| x.formula.as_ref())
&& !formula_hidden(wb, sheet, c)
{
return format!("={}", f.text);
}

94
crates/engine/src/tests.rs
View File

@ -165,6 +165,100 @@ fn sort_and_filter() {
assert_eq!(r["hiddenRows"], 1);
}
/// A sheet with locked data in A1:B3, unlocked cells in D1:D3 and row 10, then protected.
fn protected(extra: serde_json::Value) -> Session {
let mut s = s();
s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap();
s.execute("cell.set", json!({"cell": "B1", "input": "=A1+D1"})).unwrap();
s.execute("range.setValues", json!({"range": "D1", "values": [[10], [20], [30]]})).unwrap();
s.execute("home.lockCell", json!({"range": "D1:D3", "on": false})).unwrap();
s.execute("home.lockCell", json!({"range": "10:10", "on": false})).unwrap();
s.execute("review.protectSheet", extra).unwrap();
s
}
#[test]
fn protection_refuses_edits_of_locked_cells() {
let refused = |s: &mut Session, id: &str, p: serde_json::Value| {
let before = s.doc().unwrap().wb.clone();
let e = s.execute(id, p).expect_err(id).to_string();
assert!(e.contains("protected sheet"), "{id}: {e}");
assert_eq!(*s.doc().unwrap().wb, *before, "{id} changed the workbook");
};
let mut s = protected(json!({}));
refused(&mut s, "cell.set", json!({"cell": "A1", "input": "9"}));
refused(&mut s, "edit.clearContents", json!({"range": "A1:A3"}));
refused(&mut s, "edit.clearAll", json!({"range": "A1"}));
refused(&mut s, "range.setValues", json!({"range": "A2", "values": [[9]]}));
refused(&mut s, "range.fill", json!({"range": "A5:A6", "input": "x"}));
refused(&mut s, "edit.fillDown", json!({"range": "A1:A3"}));
refused(&mut s, "edit.fillRight", json!({"range": "A1:B1"}));
refused(&mut s, "edit.fillUp", json!({"range": "A1:A3"}));
refused(&mut s, "edit.fillLeft", json!({"range": "A1:B1"}));
refused(&mut s, "edit.autoFill", json!({"source": "A1", "target": "A1:A5"}));
s.execute("edit.copy", json!({"range": "D1"})).unwrap();
refused(&mut s, "edit.paste", json!({"at": "A2"}));
refused(&mut s, "edit.paste", json!({"at": "A2", "text": "x\ty"}));
refused(&mut s, "home.deleteRows", json!({"rows": "2:2"}));
refused(&mut s, "home.deleteColumns", json!({"cols": "A:A"}));
refused(&mut s, "home.insertRows", json!({"rows": "1:1"}));
refused(&mut s, "home.deleteCells", json!({"range": "A1", "shift": "up"}));
refused(&mut s, "home.insertCells", json!({"range": "A1", "shift": "down"}));
refused(&mut s, "edit.replace", json!({"what": "2", "with": "5"}));
refused(&mut s, "data.goalSeek", json!({"set": "B1", "to": 50, "changing": "A1"}));
// Unlocked cells stay editable.
s.execute("edit.paste", json!({"at": "D2"})).unwrap();
assert_eq!(v(&s, "D2"), Value::Number(10.0));
s.execute("range.setValues", json!({"range": "D2", "values": [[5]]})).unwrap();
s.execute("edit.fillDown", json!({"range": "D1:D3"})).unwrap();
assert_eq!(v(&s, "D3"), Value::Number(10.0));
s.execute("edit.clearContents", json!({"range": "D3"})).unwrap();
assert_eq!(v(&s, "D3"), Value::Empty);
s.execute("data.goalSeek", json!({"set": "B1", "to": 50, "changing": "D1"})).unwrap();
assert!((v(&s, "B1").as_f64().unwrap() - 50.0).abs() < 0.001);
s.execute("edit.replace", json!({"what": "49", "with": "48", "wholeCell": true})).unwrap();
assert_eq!(v(&s, "A1"), Value::Number(1.0));
}
#[test]
fn protection_allows_permitted_row_and_column_edits() {
let mut s = protected(json!({"insertRows": true, "deleteRows": true, "insertColumns": true}));
// Inserting shifts locked cells, which the protection allows.
s.execute("home.insertRows", json!({"rows": "1:1"})).unwrap();
assert_eq!(v(&s, "A2"), Value::Number(1.0));
s.execute("home.insertColumns", json!({"cols": "A:A"})).unwrap();
// Deleting a row with locked cells isn't; deleting an unlocked row is.
assert!(s.execute("home.deleteRows", json!({"rows": "2:2"})).is_err());
s.execute("home.deleteRows", json!({"rows": "11:11"})).unwrap();
assert!(s.execute("home.deleteColumns", json!({"cols": "Z:Z"})).is_err());
}
#[test]
fn hidden_formulas_stay_hidden_on_protected_sheets() {
let mut s = s();
s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(3)*7"})).unwrap();
s.execute("cell.set", json!({"cell": "B1", "input": "=1+1"})).unwrap();
s.execute("home.formatCells", json!({"range": "A1", "style": {"protection": {"locked": true, "hidden": true}}})).unwrap();
s.execute("formulas.showFormulas", json!({"on": true})).unwrap();
let text = |s: &Session, a: &str| {
let d = s.doc().unwrap();
let sh = d.wb.active().unwrap();
crate::display::cell_text(&d.wb, sh, CellRef::parse(a).unwrap())
};
assert_eq!(text(&s, "A1"), "=SEQUENCE(3)*7");
assert_eq!(s.execute("edit.find", json!({"what": "SEQUENCE", "all": true})).unwrap()["count"], 1);
s.execute("review.protectSheet", json!({})).unwrap();
assert_eq!(text(&s, "A1"), "7");
assert_eq!(text(&s, "B1"), "=1+1");
let d = s.doc().unwrap();
let sh = d.wb.active().unwrap();
assert!(crate::display::formula_hidden(&d.wb, sh, CellRef::parse("A2").unwrap()));
assert!(!crate::display::formula_hidden(&d.wb, sh, CellRef::parse("B1").unwrap()));
assert!(s.execute("edit.find", json!({"what": "SEQUENCE", "all": true})).is_err());
let r = s.execute("edit.find", json!({"what": "7", "all": true})).unwrap();
assert_eq!(r["results"][0]["value"], "7");
}
#[test]
fn formatting_and_styles() {
let mut s = s();

48
crates/model/src/store.rs
View File

@ -120,6 +120,40 @@ impl CellStore {
pub fn has(&self, c: CellRef) -> bool {
self.get(c).is_some_and(|cell| !cell.value.is_empty() || cell.formula.is_some())
}
/// Positions whose cells differ between `self` and `other` (stored on one side only, or not
/// equal). Bands both stores share are skipped, so comparing an edited copy with the store
/// it was cloned from only visits the bands the edit touched.
pub fn diff(&self, other: &CellStore) -> Vec<CellRef> {
let empty = Band::default();
let mut keys: Vec<u32> = self.bands.keys().chain(other.bands.keys()).copied().collect();
keys.sort_unstable();
keys.dedup();
let mut out = Vec::new();
for k in keys {
let (a, b) = (self.bands.get(&k), other.bands.get(&k));
if let (Some(a), Some(b)) = (a, b)
&& Arc::ptr_eq(a, b)
{
continue;
}
let (a, b) = (a.map_or(&empty, |x| x), b.map_or(&empty, |x| x));
for (row, cols) in &a.rows {
for (col, cell) in cols {
if b.rows.get(row).and_then(|r| r.get(col)) != Some(cell) {
out.push(CellRef::new(*row, *col));
}
}
}
for (row, cols) in &b.rows {
for col in cols.keys() {
if !a.rows.get(row).is_some_and(|r| r.contains_key(col)) {
out.push(CellRef::new(*row, *col));
}
}
}
}
out
}
/// Removes every cell in `r` and returns them.
pub fn take_range(&mut self, r: RangeRef) -> Vec<(CellRef, Cell)> {
let keys: Vec<CellRef> = self.iter_range(r).map(|(c, _)| c).collect();
@ -210,6 +244,20 @@ mod tests {
assert_eq!(s.get(c(5, 0)).unwrap().value, Value::Number(99.0));
}
#[test]
fn diff_visits_changed_bands() {
let mut s = CellStore::new();
for r in 0..1000 {
s.set(c(r, 0), Cell::value((r as f64).into()));
}
let snap = s.clone();
assert!(s.diff(&snap).is_empty());
s.set(c(5, 0), Cell::value(99.0.into()));
s.set(c(700, 3), Cell::value("new".into()));
s.remove(c(900, 0));
assert_eq!(snap.diff(&s), [c(5, 0), c(700, 3), c(900, 0)]);
}
#[test]
fn range_iteration_order() {
let mut s = CellStore::new();

3
crates/ui-egui/src/formula_bar.rs
View File

@ -77,6 +77,9 @@ fn active_input(app: &SheetApp) -> String {
if r.contains(a)
&& let Some(f) = sh.cell(*anchor).and_then(|c| c.formula.as_ref())
{
if gridcraft_engine::display::formula_hidden(&d.wb, sh, *anchor) {
return String::new();
}
return format!("={}", f.text);
}
}