From 41ba7be58ba6ab7a9cc132d45399ef5ff736767c Mon Sep 17 00:00:00 2001 From: Commanderx-code Date: Fri, 25 Sep 2026 02:41:06 -0400 Subject: [PATCH 1/5] Guard per-file diffs against nested submodule filters platform::git only added --ignore-submodules=dirty when args[0] was "status" or "diff". diff_file passes --literal-pathspecs first, so the guard was skipped and viewing a staged submodule's Unstaged diff let git run `git status` inside it, loading that repository's own clean/process filters. Leading --options are now passed through as global options and the guards apply to the real subcommand. Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/git_changes.rs | 29 +++++++++++++++++++++++++++++ src-tauri/src/platform.rs | 5 +++++ 2 files changed, 34 insertions(+) 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/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 From 487a259bd665e88ac7f9357f8d8f52e2129af96f Mon Sep 17 00:00:00 2001 From: Commanderx-code Date: Fri, 25 Sep 2026 02:41:06 -0400 Subject: [PATCH 2/5] Bound the recovery notes read recovery_notes read the settings-supplied path with an unbounded read_to_string before its 256 KB check, so a path like /dev/zero (for example from an imported setup bundle) exhausted memory. Non-regular files are now refused and the read stops at 256,001 bytes. Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/health.rs | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) 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"); + } } From 6aaf28d5d69fc6dff3c08041dcbb17e68d2fd96f Mon Sep 17 00:00:00 2001 From: Commanderx-code Date: Fri, 25 Sep 2026 02:41:06 -0400 Subject: [PATCH 3/5] Write settings.json and its backup with 0600 permissions save_settings wrote settings.json with the umask default (usually 0644) and copied the .bak with the same mode, although it can hold repository addresses, password-file names and custom commands. Both now go through platform::atomic_write like the app's other private stores. Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/settings.rs | 49 ++++++++++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 8 deletions(-) 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] From 5036c012c827d2c56c030ee88f751ebfd4e8c0ee Mon Sep 17 00:00:00 2001 From: Commanderx-code Date: Fri, 25 Sep 2026 02:41:06 -0400 Subject: [PATCH 4/5] Refuse embedded passwords in rest: Restic repositories The embedded-password check parsed rest:https://user:pass@host as an opaque "rest" URL and never saw the password, so REST-server credentials reached restic's argv. The check now also inspects the address after the rest: prefix, with the same error pointing to Restic's credential environment. Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/integrations.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) 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()); From 3481578fdc663dbe8a706253f0e8d5c03b250db3 Mon Sep 17 00:00:00 2001 From: Commanderx-code Date: Fri, 25 Sep 2026 02:43:15 -0400 Subject: [PATCH 5/5] Strip OSC sequences from job output in linear time cleanOutput's /\x1b\][^\x07]*(?:\x07|\x1b\\)/g let the body run past later ESC bytes, so a flood of unterminated "ESC ]" pairs, for example in git sideband output from a hostile remote, backtracked quadratically and froze the renderer on every launch while the job stayed in history. An indexOf scan now reproduces the regex's result exactly (first BEL after the introducer, otherwise the last ST) in linear time. Co-Authored-By: Claude Opus 5.5 --- src/feature-model.js | 24 ++++++++++++++++++++++-- tests/features.test.mjs | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) 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); +});