fix(cli): sanitize item titles in commit messages (audit I1)

Control characters (newlines, tabs) in item titles corrupted git log
output. Now strips control chars and truncates to 50 chars.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
adlee-was-taken
2026-05-02 09:23:52 -04:00
parent 81f1f8ec31
commit d6703be2b1
2 changed files with 46 additions and 6 deletions

View File

@@ -115,6 +115,21 @@ pub fn write_groups_cache(
std::fs::write(path, body)
}
/// Sanitize a string for use in a git commit message subject line.
///
/// Removes all Unicode control characters (U+0000U+001F, U+007F, and higher
/// control planes) so that newlines and escape sequences cannot corrupt `git
/// log` output. Truncates to 50 characters so the subject line stays within
/// the conventional limit.
///
/// Audit I1: item titles are user-supplied and may contain arbitrary bytes.
pub fn sanitize_for_commit(s: &str) -> String {
s.chars()
.filter(|c| !c.is_control())
.take(50)
.collect()
}
/// Decode a QR image at `path`. Returns the otpauth secret (base32) if the
/// QR decodes to an `otpauth://...` URI with a `secret` query param.
pub fn decode_totp_qr(path: &std::path::Path) -> anyhow::Result<String> {
@@ -179,6 +194,29 @@ mod tests {
assert_eq!(iso8601(1_776_556_800), "2026-04-19T00:00:00Z");
}
#[test]
fn sanitize_for_commit_strips_control_chars() {
assert_eq!(sanitize_for_commit("line1\nline2"), "line1line2");
assert_eq!(sanitize_for_commit("a\tb"), "ab");
assert_eq!(sanitize_for_commit("normal"), "normal");
assert_eq!(sanitize_for_commit("cr\r\nline"), "crline");
// ESC (U+001B) is control and gets stripped; bracket sequences are printable
assert_eq!(sanitize_for_commit("\x1b[31mred\x1b[0m"), "[31mred[0m");
}
#[test]
fn sanitize_for_commit_truncates_to_50() {
let long = "a".repeat(60);
assert_eq!(sanitize_for_commit(&long).len(), 50);
assert_eq!(sanitize_for_commit(&long), "a".repeat(50));
}
#[test]
fn sanitize_for_commit_allows_unicode() {
assert_eq!(sanitize_for_commit("cafe\u{0301}"), "cafe\u{0301}");
assert_eq!(sanitize_for_commit("emoji \u{1F4AA}"), "emoji \u{1F4AA}");
}
#[test]
fn humanize_age_buckets() {
assert_eq!(humanize_age(0), "just now");

View File

@@ -576,7 +576,7 @@ fn cmd_add(kind: AddKind) -> Result<()> {
paths.push(format!("attachments/{}/{}.enc", item.id.as_str(), att.id.as_str()));
}
let path_refs: Vec<&str> = paths.iter().map(|s| s.as_str()).collect();
commit_paths(&vault, &format!("add: {} ({})", item.title, item.id.as_str()), &path_refs)?;
commit_paths(&vault, &format!("add: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()), &path_refs)?;
eprintln!("Added: {} (id={})", item.title, item.id.as_str());
Ok(())
@@ -1121,7 +1121,7 @@ fn cmd_edit(query: String, totp_qr: Option<PathBuf>) -> Result<()> {
manifest.upsert(&item);
vault.save_manifest(&manifest)?;
refresh_groups_cache(vault.root(), &manifest);
commit_paths(&vault, &format!("edit: {} ({})", item.title, item.id.as_str()),
commit_paths(&vault, &format!("edit: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()),
&[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?;
eprintln!("Updated {}", item.id.as_str());
Ok(())
@@ -1338,7 +1338,7 @@ fn cmd_rm(query: String) -> Result<()> {
manifest.upsert(&item);
vault.save_manifest(&manifest)?;
refresh_groups_cache(vault.root(), &manifest);
commit_paths(&vault, &format!("trash: {} ({})", item.title, item.id.as_str()),
commit_paths(&vault, &format!("trash: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()),
&[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?;
eprintln!("Moved to trash: {}", item.title);
Ok(())
@@ -1356,7 +1356,7 @@ fn cmd_restore(query: String) -> Result<()> {
manifest.upsert(&item);
vault.save_manifest(&manifest)?;
refresh_groups_cache(vault.root(), &manifest);
commit_paths(&vault, &format!("restore: {} ({})", item.title, item.id.as_str()),
commit_paths(&vault, &format!("restore: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()),
&[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?;
eprintln!("Restored: {}", item.title);
Ok(())
@@ -1858,7 +1858,9 @@ fn cmd_attach(query: String, file: PathBuf) -> Result<()> {
];
let path_refs: Vec<&str> = paths.iter().map(|s| s.as_str()).collect();
commit_paths(&vault, &format!("attach: {}{} ({})",
file.display(), item.title, item.id.as_str()), &path_refs)?;
crate::helpers::sanitize_for_commit(&file.display().to_string()),
crate::helpers::sanitize_for_commit(&item.title),
item.id.as_str()), &path_refs)?;
eprintln!("Attached {} to {} (aid={})", file.display(), item.title, enc.id.as_str());
Ok(())
}
@@ -1941,7 +1943,7 @@ fn cmd_detach(query: String, aid: String) -> Result<()> {
let blob_relpath = format!("attachments/{}/{}.enc", item.id.as_str(), removed.id.as_str());
commit_paths(
&vault,
&format!("detach: {} from {} ({})", removed.filename, item.title, item.id.as_str()),
&format!("detach: {} from {} ({})", crate::helpers::sanitize_for_commit(&removed.filename), crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()),
&[&item_path, "manifest.enc", &blob_relpath],
)?;
eprintln!("Detached {} (aid={}) from {}", removed.filename, aid, item.title);