From 3e55bcdafc70f60d656a8dca3f62649bc88343f2 Mon Sep 17 00:00:00 2001 From: Owain Lewis Date: Sat, 25 Jul 2026 21:19:03 +0100 Subject: [PATCH 1/2] fix: harden runtime state and job output --- src/audit.rs | 41 +++++++++++++++++++++++++++++++----- src/jobs.rs | 46 +++++++++++++++++++++++++++++++++------- src/store.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 127 insertions(+), 19 deletions(-) diff --git a/src/audit.rs b/src/audit.rs index 0f1355f..4f7139c 100644 --- a/src/audit.rs +++ b/src/audit.rs @@ -83,15 +83,23 @@ impl AuditLog { pub fn record(&self, event: AuditEvent) -> Result<()> { let _guard = self.lock.lock().unwrap(); - if let Some(parent) = Path::new(&self.path).parent() { + let path = Path::new(&self.path); + if let Some(parent) = path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("create audit log directory {}", parent.display()))?; } - let mut file = std::fs::OpenOptions::new() - .create(true) - .append(true) - .open(&self.path) + let mut options = std::fs::OpenOptions::new(); + options.create(true).append(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options + .open(path) .with_context(|| format!("open audit log {}", self.path))?; + crate::util::restrict_permissions(path, false) + .with_context(|| format!("restrict audit log permissions {}", self.path))?; serde_json::to_writer(&mut file, &event).context("write audit event")?; use std::io::Write; writeln!(file).context("finish audit event")?; @@ -381,4 +389,27 @@ mod tests { let _ = std::fs::remove_file(path); } + + #[cfg(unix)] + #[test] + fn audit_file_is_private_and_existing_permissions_are_repaired() { + use std::os::unix::fs::PermissionsExt; + + let path = temp_path("audit-permissions"); + let audit = AuditLog::new(path.to_string_lossy().to_string(), false, "imessage"); + audit.record(audit.completed(1, "created")).unwrap(); + assert_eq!( + std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o666)).unwrap(); + audit.record(audit.completed(2, "repaired")).unwrap(); + assert_eq!( + std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + + let _ = std::fs::remove_file(path); + } } diff --git a/src/jobs.rs b/src/jobs.rs index 098a540..ddb3990 100644 --- a/src/jobs.rs +++ b/src/jobs.rs @@ -1985,17 +1985,24 @@ fn bound_result(value: &str) -> String { pub fn format_catalog_table(catalog: &Catalog) -> String { let header = ("NAME", "STATUS", "DETAIL"); - let mut rows: Vec<(String, &'static str, String)> = catalog + let mut rows: Vec<(String, String, String)> = catalog .jobs .values() - .map(|job| (job.name.clone(), "valid", job.backend.as_str().to_string())) + .map(|job| { + ( + escape_table_cell(&job.name), + "valid".to_string(), + escape_table_cell(job.backend.as_str()), + ) + }) .collect(); - rows.extend( - catalog - .errors - .iter() - .map(|error| (error.name.clone(), "invalid", error.message.clone())), - ); + rows.extend(catalog.errors.iter().map(|error| { + ( + escape_table_cell(&error.name), + "invalid".to_string(), + escape_table_cell(&error.message), + ) + })); rows.sort_by(|a, b| a.0.cmp(&b.0)); if rows.is_empty() { @@ -2028,6 +2035,10 @@ pub fn format_catalog_table(catalog: &Catalog) -> String { out } +fn escape_table_cell(value: &str) -> String { + value.chars().flat_map(char::escape_default).collect() +} + pub fn format_job(job: &Job) -> String { let evals = if job.evals.is_empty() { "none".to_string() @@ -2297,6 +2308,25 @@ mod tests { assert!(!job.triggers[0].enabled); } + #[test] + fn catalog_table_escapes_control_characters_in_invalid_jobs() { + let catalog = Catalog { + jobs: BTreeMap::new(), + errors: vec![JobError { + name: "bad\nname.md".to_string(), + path: PathBuf::from("bad\nname.md"), + message: "invalid\n\u{1b}[31mred".to_string(), + }], + }; + + let table = format_catalog_table(&catalog); + + assert_eq!(table.lines().count(), 2); + assert!(table.contains(r"bad\nname.md")); + assert!(table.contains(r"invalid\n\u{1b}[31mred")); + assert!(!table.contains('\u{1b}')); + } + #[test] fn validation_enforces_permission_timeout_backend_and_workdir() { let jobs_dir = temp_dir("jobs-validation"); diff --git a/src/store.rs b/src/store.rs index d8b1ded..1ff67ac 100644 --- a/src/store.rs +++ b/src/store.rs @@ -2,7 +2,8 @@ //! conversation thread to the active agent backend session. use std::collections::HashMap; -use std::io::ErrorKind; +use std::fs::OpenOptions; +use std::io::{ErrorKind, Write}; use std::path::PathBuf; use anyhow::{anyhow, Context, Result}; @@ -40,7 +41,11 @@ impl Store { pub fn open(path: &str) -> Result { let p = PathBuf::from(path); let state = match std::fs::read_to_string(&p) { - Ok(s) => serde_json::from_str(&s).with_context(|| format!("parse state {path}"))?, + Ok(s) => { + crate::util::restrict_permissions(&p, false) + .with_context(|| format!("restrict state permissions {path}"))?; + serde_json::from_str(&s).with_context(|| format!("parse state {path}"))? + } Err(e) if e.kind() == ErrorKind::NotFound => State::default(), Err(e) => return Err(anyhow!("read state {path}: {e}")), }; @@ -194,11 +199,29 @@ impl Store { if let Some(dir) = self.path.parent() { std::fs::create_dir_all(dir).context("create state dir")?; } - let tmp = self.path.with_extension("tmp"); + let tmp = self + .path + .with_extension(format!("tmp-{}", uuid::Uuid::new_v4())); let data = serde_json::to_string_pretty(&self.state)?; - std::fs::write(&tmp, data).context("write state")?; - std::fs::rename(&tmp, &self.path).context("rename state")?; - Ok(()) + let result = (|| -> Result<()> { + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options.open(&tmp).context("create temporary state")?; + crate::util::restrict_permissions(&tmp, false) + .context("restrict temporary state permissions")?; + file.write_all(data.as_bytes()).context("write state")?; + std::fs::rename(&tmp, &self.path).context("rename state")?; + Ok(()) + })(); + if result.is_err() { + let _ = std::fs::remove_file(&tmp); + } + result } #[cfg(test)] @@ -416,4 +439,28 @@ mod tests { assert!(!raw.contains("\"dm:+15551234567\"")); let _ = std::fs::remove_file(path); } + + #[cfg(unix)] + #[test] + fn state_file_is_private_and_existing_permissions_are_repaired() { + use std::os::unix::fs::PermissionsExt; + + let path = temp_state_path(); + let mut store = Store::open(&path).unwrap(); + store.set_cursor("imessage", 1).unwrap(); + assert_eq!( + std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o666)).unwrap(); + drop(store); + Store::open(&path).unwrap(); + assert_eq!( + std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + + let _ = std::fs::remove_file(path); + } } From 737986847fb2aa300fab4d09580c805e18bd0a47 Mon Sep 17 00:00:00 2001 From: Owain Lewis Date: Sat, 25 Jul 2026 23:58:03 +0100 Subject: [PATCH 2/2] fix(jobs): preserve printable table text --- src/jobs.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/jobs.rs b/src/jobs.rs index ddb3990..df70d57 100644 --- a/src/jobs.rs +++ b/src/jobs.rs @@ -2036,7 +2036,15 @@ pub fn format_catalog_table(catalog: &Catalog) -> String { } fn escape_table_cell(value: &str) -> String { - value.chars().flat_map(char::escape_default).collect() + let mut escaped = String::with_capacity(value.len()); + for character in value.chars() { + if character.is_control() { + escaped.extend(character.escape_default()); + } else { + escaped.push(character); + } + } + escaped } pub fn format_job(job: &Job) -> String { @@ -2315,7 +2323,7 @@ mod tests { errors: vec![JobError { name: "bad\nname.md".to_string(), path: PathBuf::from("bad\nname.md"), - message: "invalid\n\u{1b}[31mred".to_string(), + message: "invalid \"value\" at café\\repo\n\u{1b}[31mred".to_string(), }], }; @@ -2323,7 +2331,8 @@ mod tests { assert_eq!(table.lines().count(), 2); assert!(table.contains(r"bad\nname.md")); - assert!(table.contains(r"invalid\n\u{1b}[31mred")); + assert!(table.contains("invalid \"value\" at café\\repo")); + assert!(table.contains(r"\n\u{1b}[31mred")); assert!(!table.contains('\u{1b}')); }