From 9aa9eca9e827b9b00f74b9908fe4ec69b60bc3fd Mon Sep 17 00:00:00 2001 From: Mat Strange Date: Tue, 20 Oct 2026 13:35:55 +0100 Subject: [PATCH] Restrict Editing: the post-check compares the settings and the metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `outside_unchanged` is the check that rolls a command back when it changed anything outside the editable regions. It compared the text, the formatting, the page setup, the document defaults and the styles — but not the document's settings, its metadata, its custom properties, the resolved flag of its comments, its list definitions or its bibliography sources. A command that changed the metadata of a protected document was accepted and never rolled back: `file.properties {"title": "Forged title"}` on a protected document with an exception changed the title and stayed. The settings, the core properties, the custom properties, the comments' resolved flags, the list definitions and the sources are compared now, so a command that changes them is rolled back like any other change outside the editable regions. The revisions and the media store stay out of it on purpose: an edit inside an exception legitimately leaves a revision, and a picture inserted there adds to the media store. The list definitions and the sources are compared like the styles: the ones the document had must be unchanged, but a new one may be added — a list or a citation inserted inside an editable region adds its definition, like applying a style adds one. Saving is unaffected: it writes the metadata, but `file.save` is a pure command and isn't post-checked, and the protection itself is changed by `review.restrict`, which is always allowed. Co-Authored-By: opencode --- crates/engine/src/cmd/protect.rs | 56 +++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/protect.rs b/crates/engine/src/cmd/protect.rs index 71972079a..b05fc193c 100644 --- a/crates/engine/src/cmd/protect.rs +++ b/crates/engine/src/cmd/protect.rs @@ -370,7 +370,7 @@ enum HoleMode { /// run formatting and paragraph formatting outside the ranges `author` may edit, in every story, /// and the formatting of every table, row and cell that holds any of that protected content, and what /// reaches the whole document: page setup, the document defaults and the existing styles -/// (applying a style may add one). +/// (applying a style may add one), the document's settings and its metadata. pub(crate) fn outside_unchanged(before: &Document, after: &Document, author: &str) -> bool { // Protection can't change mid-command (`protection_lifted` guards undo and redo). let mode = if Some("forms") == before.settings.protection.as_deref() { HoleMode::Controls } else { HoleMode::Perms(author.into()) }; @@ -380,9 +380,35 @@ pub(crate) fn outside_unchanged(before: &Document, after: &Document, author: &st && b.default_chr == a.default_chr && b.default_para == a.default_para && b.styles.iter().all(|st| a.get(&st.id) == Some(st)) + && before.settings == after.settings + && before.core == after.core + && before.custom_props == after.custom_props + && comments_unchanged(before, after) + && lists_unchanged(before, after) + && sources_unchanged(before, after) && outside(before, &mode) == outside(after, &mode) } +/// The comments' resolved flags: a comment's text is a part, and the parts are compared with the +/// rest of the document; the flag is the comment's own state. +fn comments_unchanged(before: &Document, after: &Document) -> bool { + before.comments.iter().all(|(id, c)| after.comments.get(id).is_some_and(|a| a.resolved == c.resolved)) +} + +/// The list definitions: the ones the document had must be unchanged — a list's format reaches +/// every paragraph that uses it — but a new one may be added, like a new style: applying a list +/// inside an editable region adds its definition. +fn lists_unchanged(before: &Document, after: &Document) -> bool { + before.numbering.abstracts.iter().all(|x| after.numbering.abstracts.contains(x)) + && before.numbering.nums.iter().all(|x| after.numbering.nums.contains(x)) +} + +/// The bibliography sources: the ones the document had must be unchanged; a new one may be added +/// (a citation inserted inside an editable region). +fn sources_unchanged(before: &Document, after: &Document) -> bool { + before.sources.iter().all(|x| after.sources.contains(x)) +} + /// The section properties paragraphs end, in order. fn sections(doc: &Document) -> Vec<&wordcraft_doc::SectionProps> { doc.para_paths(StoryRef::Body).iter().filter_map(|path| doc.para(StoryRef::Body, path)?.section.as_deref()).collect() @@ -835,6 +861,34 @@ mod tests { assert!(t.doc.settings.protect_formatting); } + /// The settings and the metadata of a protected document are part of what stays as it was: a + /// command that changes them is rolled back, like any other change outside the editable + /// regions (#1327 follow-up). + #[test] + fn a_protected_document_keeps_its_settings_and_metadata() { + let mut s = Session::new(Document::from_text("Locked intro.\nFill in here.")); + select(&mut s, "Fill in here."); + s.run("review.restrict", &json!({"exception": "add"})).unwrap(); + s.run("review.restrict", &json!({"mode": "readOnly"})).unwrap(); + let before = s.doc.clone(); + // The properties: the change is rolled back, and the command says so. + let e = s.run("file.properties", &json!({"title": "Forged title"})).unwrap_err().to_string(); + assert!(e.contains("only the editable regions"), "{e}"); + assert_eq!(s.doc, before); + assert_ne!(s.doc.core.title, "Forged title"); + // A custom property too. + assert!(s.run("file.properties", &json!({"custom": {"Owner": "Alice"}})).is_err()); + assert_eq!(s.doc, before); + // An edit inside the exception still works, and isn't rolled back. + select(&mut s, "here"); + s.run("text.insert", &json!({"text": "there"})).unwrap(); + assert!(s.doc.plain_text(StoryRef::Body).contains("Fill in there.")); + // Without protection the same command works. + let mut t = Session::new(Document::from_text("Some text.")); + t.run("file.properties", &json!({"title": "A title"})).unwrap(); + assert_eq!(t.doc.core.title, "A title"); + } + /// Reading the document stays allowed under protection: the inspector without removals, the /// privacy settings, the info, and saving a version snapshot. #[test]