diff --git a/src-tauri/src/git_changes.rs b/src-tauri/src/git_changes.rs index f72c7ad..5216e67 100644 --- a/src-tauri/src/git_changes.rs +++ b/src-tauri/src/git_changes.rs @@ -267,6 +267,35 @@ mod tests { assert!(diff_file(path, "new", false).unwrap()["text"].as_str().unwrap().contains("+new content")); assert!(diff_file(path, "../outside", false).is_err()); } + #[test] + fn file_diffs_skip_nested_submodule_filters() { + use std::os::unix::fs::PermissionsExt; + let raw = |dir: &Path, args: &[&str]| { + let output = Command::new("git").arg("-C").arg(dir).args(args).output().unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + }; + let dir = fixture(); let path = dir.path(); + let child = path.join("child"); + fs::create_dir(&child).unwrap(); + raw(&child, &["init"]); + fs::write(child.join("tracked"), "original\n").unwrap(); + raw(&child, &["add", "tracked"]); + raw(&child, &["-c", "user.name=Fixture", "-c", "user.email=fixture@example.invalid", "-c", "commit.gpgsign=false", "commit", "-m", "initial"]); + raw(path, &["add", "child"]); + let hook = child.join("filter-hook"); + fs::write(&hook, "#!/bin/sh\ntouch nested-filter-ran\ncat\n").unwrap(); + fs::set_permissions(&hook, fs::Permissions::from_mode(0o700)).unwrap(); + fs::write(child.join(".gitattributes"), "tracked filter=nested\n").unwrap(); + raw(&child, &["config", "filter.nested.clean", hook.to_str().unwrap()]); + fs::write(child.join("tracked"), "modified\n").unwrap(); + // Positive attack control: ordinary Git recurses into the submodule. + raw(path, &["--literal-pathspecs", "diff", "--no-ext-diff", "--no-textconv", "--", "child"]); + assert!(child.join("nested-filter-ran").exists(), "nested filter fixture did not execute"); + fs::remove_file(child.join("nested-filter-ran")).unwrap(); + diff_file(path, "child", false).unwrap(); + diff_file(path, "child", true).unwrap(); + assert!(!child.join("nested-filter-ran").exists(), "nested filter executed during file diff"); + } #[test] fn stash_apply_retains_stash_and_publish_sets_upstream() { diff --git a/src-tauri/src/health.rs b/src-tauri/src/health.rs index 629272c..a48595b 100644 --- a/src-tauri/src/health.rs +++ b/src-tauri/src/health.rs @@ -127,11 +127,25 @@ pub async fn system_health(app: tauri::AppHandle) -> Result { pub fn recovery_notes(app: tauri::AppHandle) -> Result { let s = crate::settings::load_settings(app)?.unwrap_or_default(); let path = p::expand(&s.integrations.recovery_notes_path)?; - let text = fs::read_to_string(path).map_err(|e| e.to_string())?; - if text.len() > 256_000 { + read_recovery_notes(&path) +} +fn read_recovery_notes(path: &std::path::Path) -> Result { + use std::io::Read; + // Stat before opening so devices, FIFOs and sockets are refused without blocking or + // streaming forever; the bounded read below caps memory even if the path is swapped. + if !fs::metadata(path).map_err(|e| e.to_string())?.is_file() { + return Err("Recovery notes must be a regular file".into()); + } + let mut bytes = Vec::new(); + fs::File::open(path) + .map_err(|e| e.to_string())? + .take(256_001) + .read_to_end(&mut bytes) + .map_err(|e| e.to_string())?; + if bytes.len() > 256_000 { return Err("Recovery notes exceed 256 KB; open them in your editor".into()); } - Ok(text) + std::io::read_to_string(bytes.as_slice()).map_err(|e| e.to_string()) } #[cfg(test)] @@ -164,4 +178,18 @@ mod readiness_tests { assert_eq!(&plan.args[plan.args.len()-2..],&["cat","config"]); assert_eq!(plan.timeout_seconds,120); } + #[test] + fn recovery_notes_refuse_devices_and_bound_oversized_files(){ + assert_eq!(read_recovery_notes(std::path::Path::new("/dev/zero")).unwrap_err(),"Recovery notes must be a regular file"); + let temp=tempfile::tempdir().unwrap(); + let notes=temp.path().join("notes.md"); + fs::write(¬es,"Restore steps\n").unwrap(); + assert_eq!(read_recovery_notes(¬es).unwrap(),"Restore steps\n"); + fs::write(¬es,"x".repeat(256_000)).unwrap(); + assert_eq!(read_recovery_notes(¬es).unwrap().len(),256_000); + fs::write(¬es,format!("{}é","x".repeat(255_999))).unwrap(); + assert_eq!(read_recovery_notes(¬es).unwrap_err(),"Recovery notes exceed 256 KB; open them in your editor"); + fs::write(¬es,[b'a',0xff]).unwrap(); + assert_eq!(read_recovery_notes(¬es).unwrap_err(),"stream did not contain valid UTF-8"); + } } diff --git a/src-tauri/src/integrations.rs b/src-tauri/src/integrations.rs index 6361216..71bbb89 100644 --- a/src-tauri/src/integrations.rs +++ b/src-tauri/src/integrations.rs @@ -193,9 +193,14 @@ pub fn restic_args(i: &Integrations) -> Result, String> { if i.restic_repository.is_empty() { return Err("Configure the Restic repository in Settings".into()); } - if let Ok(url) = tauri::Url::parse(&i.restic_repository) { - if url.password().is_some() { - return Err("Use Restic's credential environment or password file instead of a password embedded in the repository URL".into()); + // Restic's REST backend nests the server URL (and any credentials) behind a + // `rest:` prefix, which URL parsing alone treats as an opaque path. + let nested = i.restic_repository.strip_prefix("rest:"); + for address in std::iter::once(i.restic_repository.as_str()).chain(nested) { + if let Ok(url) = tauri::Url::parse(address) { + if url.password().is_some() { + return Err("Use Restic's credential environment or password file instead of a password embedded in the repository URL".into()); + } } } let repository = if i.restic_repository.starts_with("~/") { @@ -522,6 +527,24 @@ mod tests { assert_eq!(&plan.args[5..7], ["--repo", "/backup/repo"]); } #[test] + fn rejects_passwords_embedded_in_repository_addresses() { + let password = tempfile::NamedTempFile::new().unwrap(); + let mut i = Settings::default().integrations; + i.restic_password_file = password.path().to_string_lossy().into_owned(); + for repository in [ + "https://backup:S3cret@nas:8000/laptop", + "rest:https://backup:S3cret@nas:8000/laptop", + "rest:http://backup:S3cret@nas:8000/", + "rest://backup:S3cret@nas:8000/", + ] { + i.restic_repository = repository.into(); + let err = restic_args(&i).unwrap_err(); + assert!(err.contains("embedded in the repository URL"), "{repository}: {err}"); + } + i.restic_repository = "rest:https://nas:8000/laptop".into(); + assert_eq!(&restic_args(&i).unwrap()[..2], ["--repo", "rest:https://nas:8000/laptop"]); + } + #[test] fn rejects_option_like_snapshot_ids() { assert!(snapshot_id("--delete").is_err()); assert!(snapshot_id("a1b2c3d4").is_ok()); diff --git a/src-tauri/src/platform.rs b/src-tauri/src/platform.rs index 32e646c..e424b0f 100644 --- a/src-tauri/src/platform.rs +++ b/src-tauri/src/platform.rs @@ -224,6 +224,11 @@ pub fn git(path: &Path, args: &[&str]) -> Result { command.arg("-c").arg(format!("filter.{name}.{setting}")); } } + // Leading global options (e.g. --literal-pathspecs) precede the subcommand; + // the guards below must apply to the subcommand Git actually runs. + let globals = args.iter().take_while(|arg| arg.starts_with("--")).count(); + let (global_args, args) = args.split_at(globals); + command.args(global_args); if let Some((subcommand, rest)) = args.split_first() { command.arg(subcommand); // A nested worktree has its own filters. Report gitlink changes without diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index da0adba..3b2f763 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -1,5 +1,8 @@ use serde::{Deserialize, Serialize}; -use std::{fs, path::PathBuf}; +use std::{ + fs, + path::{Path, PathBuf}, +}; use tauri::Manager; #[derive(Clone, Debug, Serialize, Deserialize)] @@ -216,16 +219,19 @@ pub fn load_settings(app: tauri::AppHandle) -> Result, String> pub fn save_settings(app: tauri::AppHandle, settings: Settings) -> Result<(), String> { let _setup_guard = crate::setup::WRITE_LOCK.lock().map_err(|e| e.to_string())?; settings.validate()?; - let path = location(&app)?; + write_settings(&location(&app)?, &settings) +} +// Settings can hold repository credentials, so the file and its backup are +// written privately (0600) regardless of umask or the previous file's mode. +fn write_settings(path: &Path, settings: &Settings) -> Result<(), String> { fs::create_dir_all(path.parent().ok_or("Invalid settings path")?).map_err(|e| e.to_string())?; - let data = serde_json::to_vec_pretty(&settings).map_err(|e| e.to_string())?; - // Preserve previous contents; rename in the same directory for atomic replacement on Linux. + let data = serde_json::to_vec_pretty(settings).map_err(|e| e.to_string())?; + // Preserve previous contents; atomic_write renames in the same directory for atomic replacement on Linux. if path.exists() { - fs::copy(&path, path.with_extension("json.bak")).map_err(|e| e.to_string())?; + let previous = fs::read(path).map_err(|e| e.to_string())?; + crate::platform::atomic_write(&path.with_extension("json.bak"), &previous)?; } - let temporary = path.with_extension("json.tmp"); - fs::write(&temporary, data).map_err(|e| e.to_string())?; - fs::rename(&temporary, &path).map_err(|e| e.to_string()) + crate::platform::atomic_write(path, &data) } #[cfg(test)] @@ -267,6 +273,33 @@ mod tests { settings.roots.clear(); assert!(settings.validate().is_ok()); } + #[test] + fn saved_settings_and_backup_are_private() { + use std::os::unix::fs::PermissionsExt; + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("settings.json"); + let backup = path.with_extension("json.bak"); + // Files left by earlier versions were created with umask-default modes. + for file in [&path, &backup] { + fs::write(file, "{}").unwrap(); + fs::set_permissions(file, fs::Permissions::from_mode(0o644)).unwrap(); + } + let mut settings = Settings::default(); + settings.integrations.restic_repository = "rest:https://user:secret@example.invalid/repo".into(); + write_settings(&path, &settings).unwrap(); + for file in [&path, &backup] { + assert_eq!(fs::metadata(file).unwrap().permissions().mode() & 0o777, 0o600); + } + assert_eq!(fs::read_to_string(&backup).unwrap(), "{}"); + let saved: Settings = serde_json::from_slice(&fs::read(&path).unwrap()).unwrap(); + assert_eq!(saved.integrations.restic_repository, settings.integrations.restic_repository); + let mut names: Vec<_> = fs::read_dir(temp.path()) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .collect(); + names.sort(); + assert_eq!(names, ["settings.json", "settings.json.bak"]); + } } #[tauri::command] diff --git a/src/feature-model.js b/src/feature-model.js index 094bc88..34a138c 100644 --- a/src/feature-model.js +++ b/src/feature-model.js @@ -1,9 +1,29 @@ export function backupRecordFailed(record) { return record?.success===false || (typeof record?.exit_code==='number'&&record.exit_code!==0) || ['failed','failure','error','canceled','cancelled'].includes(String(record?.status||'').toLowerCase()); } +// Strips OSC sequences exactly as /\x1b\][^\x07]*(?:\x07|\x1b\\)/g did: each ends at the +// first BEL after it, otherwise at the last ST. A scan keeps unterminated floods from +// remote job output linear where that regex backtracked quadratically. +function stripOsc(value) { + let out = "", pos = 0; + for (;;) { + const start = value.indexOf("\x1b]", pos); + if (start < 0) break; + const bel = value.indexOf("\x07", start + 2); + let end; + if (bel >= 0) end = bel + 1; + else { + const st = value.lastIndexOf("\x1b\\"); + if (st < start + 2) break; + end = st + 2; + } + out += value.slice(pos, start); + pos = end; + } + return out + value.slice(pos); +} export function cleanOutput(value = "") { - return value - .replace(/\x1b\][^\x07]*(?:\x07|\x1b\\)/g, "") + return stripOsc(value) .replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, "") .replace(/\r(?!\n)/g, "\n"); } diff --git a/tests/features.test.mjs b/tests/features.test.mjs index f84f8d8..b3c03f6 100644 --- a/tests/features.test.mjs +++ b/tests/features.test.mjs @@ -15,6 +15,7 @@ import { parseFileHistory, formatBytes, parseSnapshotFiles, + cleanOutput, } from "../src/feature-model.js"; test("integration preferences survive migration and retain explicit paths", () => { @@ -146,3 +147,21 @@ test("byte sizes are human readable", () => { assert.equal(formatBytes(5 * 1024 ** 3), "5.0 GiB"); assert.equal(formatBytes(null), "—"); }); + +test("job output cleaning strips terminal sequences as before", () => { + assert.equal(cleanOutput("a\x1b]0;title\x07b"), "ab"); + assert.equal(cleanOutput("a\x1b]0;title\x1b\\b"), "ab"); + assert.equal(cleanOutput("a\x1b]0;t\x1b\\visible\x07b"), "ab"); + assert.equal(cleanOutput("see \x1b]8;;https://example.com\x1b\\label\x1b]8;;\x1b\\ now"), "see now"); + assert.equal(cleanOutput("a\x1b]0;broken\x1b]0;title\x07b"), "ab"); + assert.equal(cleanOutput("a\x1b]0;unterminated"), "a\x1b]0;unterminated"); + assert.equal(cleanOutput("\x1b[31mred\x1b[0m\rline\r\n"), "red\nline\r\n"); + assert.equal(cleanOutput(), ""); +}); + +test("job output cleaning stays linear on unterminated OSC floods", () => { + const flood = "\x1b]".repeat(1_000_000); + const started = Date.now(); + assert.equal(cleanOutput(flood), flood); + assert.ok(Date.now() - started < 2000); +});