Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions src/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;
Expand Down Expand Up @@ -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);
}
}
55 changes: 47 additions & 8 deletions src/jobs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -2028,6 +2035,18 @@ pub fn format_catalog_table(catalog: &Catalog) -> String {
out
}

fn escape_table_cell(value: &str) -> String {
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 {
let evals = if job.evals.is_empty() {
"none".to_string()
Expand Down Expand Up @@ -2297,6 +2316,26 @@ 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 \"value\" at café\\repo\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("invalid \"value\" at café\\repo"));
assert!(table.contains(r"\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");
Expand Down
59 changes: 53 additions & 6 deletions src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -40,7 +41,11 @@ impl Store {
pub fn open(path: &str) -> Result<Store> {
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}")),
};
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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);
}
}
Loading