From d809c838c4b10bdbabeb75ba8593589adf4fef39 Mon Sep 17 00:00:00 2001 From: Mat Strange Date: Tue, 20 Oct 2026 17:20:53 +0100 Subject: [PATCH] The CLI's --author names the session, it doesn't change the document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--author` dispatched `file.setAuthor`, which a protected document refuses now that the name is checked as the identity its exceptions are matched against — so a script that named itself on a protected file failed before it ran a single command. The flag names the session, not the open document, so it sets the name directly, with the same normalisation and blank-name check the command uses. `file.setAuthor` keeps its protection check for the callers that mean to change the document's identity. Co-Authored-By: opencode --- apps/wordcraft-cli/src/main.rs | 13 ++++++++++++- crates/engine/src/cmd/file.rs | 22 ++++++++++++++-------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/apps/wordcraft-cli/src/main.rs b/apps/wordcraft-cli/src/main.rs index 446b902fa..1a273dda4 100644 --- a/apps/wordcraft-cli/src/main.rs +++ b/apps/wordcraft-cli/src/main.rs @@ -180,8 +180,12 @@ fn listed_text(doc: &wordcraft_doc::Document) -> Result { } /// `--author NAME`: the author recorded on tracked changes and comments (`file.setAuthor`). +/// The name is set on the session directly: the caller is naming itself, not changing the open +/// document, so the protection that refuses `file.setAuthor` doesn't apply here. fn set_author(s: &mut Session, name: &str) -> Result<(), String> { - s.run("file.setAuthor", &json!({"name": name})).map(|_| ()).map_err(|e| format!("--author: {e}")) + let name = wordcraft_engine::cmd::file::normalize_author(name).map_err(|e| format!("--author: {e}"))?; + s.author = name; + Ok(()) } fn arg_value(args: &[String], name: &str) -> Option { @@ -804,6 +808,13 @@ mod tests { assert!(run(&args(&["run", "--author", "Claude", "--cmd", "review.trackChanges={\"value\":true}"])).is_ok()); // The app's user name is the person's own; a bridge flag must not rewrite it. assert!(run(&args(&["mcp", "--connect", "127.0.0.1:9", "--author", "Claude"])).is_err_and(|e| e.to_string().contains("--author"))); + // The flag names the session, so a protected document doesn't refuse it — changing the + // document's identity is a different thing, and that is still refused. + let mut s = Session::new(wordcraft_doc::Document::new()); + s.run("review.restrict", &json!({"mode": "readOnly"})).unwrap(); + assert!(set_author(&mut s, "Claude").is_ok()); + assert_eq!(s.author, "Claude"); + assert!(s.run("file.setAuthor", &json!({"name": "Claude"})).is_err()); } #[test] diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 187587c47..167d0dbbc 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -14,18 +14,24 @@ pub const FORMAT_FLAT_OPC: &str = "flatopc"; pub const MAX_AUTHOR_CHARS: usize = 255; -/// Sets the user name that new tracked changes and comments are recorded under. The name is -/// trimmed, stripped of control characters (they can't go into the saved XML) and cut to -/// [`MAX_AUTHOR_CHARS`]; a blank name is refused so revisions always carry an author. -/// A protected document refuses it: the name is the identity its exceptions are checked against. -pub(crate) fn set_author(s: &mut Session, v: &Value) -> CmdResult { - super::protect::check_user_name(s)?; - let name: String = p::req_str(v, "name")?.trim().chars().filter(|c| !c.is_control()).take(MAX_AUTHOR_CHARS).collect(); +/// The user name as it is stored: trimmed, stripped of control characters (they can't go into +/// the saved XML) and cut to [`MAX_AUTHOR_CHARS`]; a blank name is refused so revisions always +/// carry an author. +pub fn normalize_author(name: &str) -> Result { + let name: String = name.trim().chars().filter(|c| !c.is_control()).take(MAX_AUTHOR_CHARS).collect(); let name = name.trim_end(); if name.is_empty() { return Err(CmdError::Params("`name` must not be blank".into())); } - s.author = name.to_string(); + Ok(name.to_string()) +} + +/// Sets the user name that new tracked changes and comments are recorded under. The name is +/// normalised by [`normalize_author`]. +/// A protected document refuses it: the name is the identity its exceptions are checked against. +pub(crate) fn set_author(s: &mut Session, v: &Value) -> CmdResult { + super::protect::check_user_name(s)?; + s.author = normalize_author(p::req_str(v, "name")?)?; let mut r = sel_result(s)?; if let Some(o) = r.as_object_mut() { o.insert("name".into(), json!(s.author));