From 4929f11b6d94c863bde8b8e06343d8bd4f4d7b21 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Sat, 25 Apr 2026 00:48:53 +0200 Subject: [PATCH] fix: harden windows release and git subprocesses --- .github/workflows/release-stable.yml | 11 ++ .github/workflows/release.yml | 12 ++ src-tauri/src/git/clone.rs | 4 +- src-tauri/src/git/commit.rs | 18 +-- src-tauri/src/git/conflict.rs | 37 ++--- src-tauri/src/git/connect.rs | 12 +- src-tauri/src/git/dates.rs | 4 +- src-tauri/src/git/history.rs | 44 +++--- src-tauri/src/git/mod.rs | 37 ++--- src-tauri/src/git/pulse.rs | 148 ++++++++++--------- src-tauri/src/git/remote.rs | 129 ++++++++++------ src-tauri/src/git/status.rs | 103 ++++++++----- src-tauri/src/lib.rs | 21 +++ src-tauri/src/mcp.rs | 17 ++- src-tauri/src/vault/cache.rs | 16 +- src-tauri/src/vault/getting_started.rs | 7 +- src-tauri/src/vault/rename.rs | 2 +- src/hooks/useGettingStartedClone.test.ts | 2 +- src/hooks/useOnboarding.test.ts | 2 +- src/utils/gettingStartedVault.test.ts | 14 +- src/utils/gettingStartedVault.ts | 44 +++++- tests/smoke/getting-started-template.spec.ts | 2 +- 22 files changed, 426 insertions(+), 260 deletions(-) diff --git a/.github/workflows/release-stable.yml b/.github/workflows/release-stable.yml index cd7ec449..8938adf4 100644 --- a/.github/workflows/release-stable.yml +++ b/.github/workflows/release-stable.yml @@ -374,6 +374,11 @@ jobs: - name: Install frontend dependencies run: pnpm install --frozen-lockfile + - name: Clear cached Windows bundle artifacts + shell: pwsh + run: | + Remove-Item -Recurse -Force -ErrorAction SilentlyContinue "src-tauri/target/x86_64-pc-windows-msvc/release/bundle" + - name: Set version shell: pwsh run: | @@ -425,6 +430,12 @@ jobs: echo "::error::Windows build produced no installable NSIS or MSI bundle." exit 1 fi + for installer in "${installers[@]}"; do + if [[ "$(basename "$installer")" != *"${{ needs.version.outputs.version }}"* ]]; then + echo "::error::Windows build produced an installer for a different version: $(basename "$installer")" + exit 1 + fi + done if [ ${#signatures[@]} -eq 0 ]; then echo "::error::Windows build produced no updater signature (.sig) artifact." exit 1 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 285b6558..974dfbe1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -341,6 +341,11 @@ jobs: - name: Install frontend dependencies run: pnpm install --frozen-lockfile + - name: Clear cached Windows bundle artifacts + shell: pwsh + run: | + Remove-Item -Recurse -Force -ErrorAction SilentlyContinue "src-tauri/target/x86_64-pc-windows-msvc/release/bundle" + - name: Set version shell: pwsh run: | @@ -392,6 +397,12 @@ jobs: echo "::error::Windows build produced no installable NSIS or MSI bundle." exit 1 fi + for installer in "${installers[@]}"; do + if [[ "$(basename "$installer")" != *"${{ needs.version.outputs.version }}"* ]]; then + echo "::error::Windows build produced an installer for a different version: $(basename "$installer")" + exit 1 + fi + done if [ ${#signatures[@]} -eq 0 ]; then echo "::error::Windows build produced no updater signature (.sig) artifact." exit 1 @@ -410,6 +421,7 @@ jobs: src-tauri/target/x86_64-pc-windows-msvc/release/bundle/msi/*.msi.sig src-tauri/target/x86_64-pc-windows-msvc/release/bundle/msi/*.zip src-tauri/target/x86_64-pc-windows-msvc/release/bundle/msi/*.zip.sig + if-no-files-found: error retention-days: 1 # ───────────────────────────────────────────────────────────── # Phase 3: Publish GitHub Release diff --git a/src-tauri/src/git/clone.rs b/src-tauri/src/git/clone.rs index e432cf2d..a9d0bfb5 100644 --- a/src-tauri/src/git/clone.rs +++ b/src-tauri/src/git/clone.rs @@ -1,6 +1,8 @@ use std::path::Path; use std::process::{Command, Output, Stdio}; +use super::git_command; + struct CloneRequest<'a> { url: &'a str, dest: &'a Path, @@ -92,7 +94,7 @@ fn run_clone(request: &CloneRequest<'_>) -> Result<(), String> { } fn build_clone_command(request: &CloneRequest<'_>, destination: &str) -> Command { - let mut command = Command::new("git"); + let mut command = git_command(); command .args(["clone", "--quiet", request.url, destination]) .env("GIT_TERMINAL_PROMPT", "0") diff --git a/src-tauri/src/git/commit.rs b/src-tauri/src/git/commit.rs index 658d7996..aa070eb1 100644 --- a/src-tauri/src/git/commit.rs +++ b/src-tauri/src/git/commit.rs @@ -1,5 +1,5 @@ +use super::git_command; use std::path::Path; -use std::process::Command; struct CommitFailure { stdout: String, @@ -11,7 +11,7 @@ pub fn git_commit(vault_path: &str, message: &str) -> Result { let vault = Path::new(vault_path); // Stage all changes - let add = Command::new("git") + let add = git_command() .args(["add", "-A"]) .current_dir(vault) .output() @@ -37,7 +37,7 @@ pub fn git_commit(vault_path: &str, message: &str) -> Result { } fn run_commit(vault: &Path, message: &str, disable_signing: bool) -> Result { - let mut command = Command::new("git"); + let mut command = git_command(); if disable_signing { command.args(["-c", "commit.gpgsign=false"]); } @@ -85,10 +85,10 @@ fn is_commit_signing_failure(detail: &str) -> bool { #[cfg(test)] mod tests { + use super::git_command; use super::*; use crate::git::tests::setup_git_repo; use std::fs; - use std::process::Command; #[test] fn test_git_commit() { @@ -101,7 +101,7 @@ mod tests { assert!(result.is_ok()); // Verify the commit exists - let log = Command::new("git") + let log = git_command() .args(["log", "--oneline", "-1"]) .current_dir(vault) .output() @@ -135,12 +135,12 @@ mod tests { let vault = dir.path(); let vp = vault.to_str().unwrap(); - Command::new("git") + git_command() .args(["config", "commit.gpgsign", "true"]) .current_dir(vault) .output() .unwrap(); - Command::new("git") + git_command() .args(["config", "gpg.program", "/missing/tolaria-test-gpg"]) .current_dir(vault) .output() @@ -153,14 +153,14 @@ mod tests { "commit should retry unsigned when signing helper is missing: {result:?}" ); - let log = Command::new("git") + let log = git_command() .args(["log", "--oneline", "-1"]) .current_dir(vault) .output() .unwrap(); assert!(String::from_utf8_lossy(&log.stdout).contains("Commit with broken signing config")); - let config = Command::new("git") + let config = git_command() .args(["config", "commit.gpgsign"]) .current_dir(vault) .output() diff --git a/src-tauri/src/git/conflict.rs b/src-tauri/src/git/conflict.rs index b8023d4b..567756f2 100644 --- a/src-tauri/src/git/conflict.rs +++ b/src-tauri/src/git/conflict.rs @@ -1,7 +1,6 @@ use std::path::Path; -use std::process::Command; -use super::run_git; +use super::{git_command, run_git}; /// List files with merge conflicts (unmerged paths). /// @@ -10,7 +9,7 @@ use super::run_git; /// stale (e.g. after a reboot or when MERGE_HEAD is missing). pub fn get_conflict_files(vault_path: &str) -> Result, String> { let vault = Path::new(vault_path); - let output = Command::new("git") + let output = git_command() .args(["ls-files", "--unmerged"]) .current_dir(vault) .output() @@ -93,13 +92,13 @@ pub fn git_commit_conflict_resolution(vault_path: &str) -> Result Command::new("git") + "rebase" => git_command() .args(["rebase", "--continue"]) .env("GIT_EDITOR", "true") .current_dir(vault) .output() .map_err(|e| format!("Failed to run git rebase --continue: {}", e))?, - _ => Command::new("git") + _ => git_command() .args(["commit", "-m", "Resolve merge conflicts"]) .current_dir(vault) .output() @@ -131,7 +130,6 @@ mod tests { use crate::git::tests::{setup_git_repo, setup_remote_pair}; use crate::git::{git_commit, git_pull, git_push}; use std::fs; - use std::process::Command; use tempfile::TempDir; #[test] @@ -203,35 +201,30 @@ mod tests { (bare_dir, clone_a_dir, clone_b_dir) } - #[test] - fn test_resolve_conflict_ours() { + fn assert_resolve_conflict_strategy(strategy: &str, expected_content: &str) { let (_bare, _clone_a, clone_b) = setup_conflict_pair(); let vp_b = clone_b.path().to_str().unwrap(); let conflicts = get_conflict_files(vp_b).unwrap(); assert!(conflicts.contains(&"conflict.md".to_string())); - git_resolve_conflict(vp_b, "conflict.md", "ours").unwrap(); + git_resolve_conflict(vp_b, "conflict.md", strategy).unwrap(); let remaining = get_conflict_files(vp_b).unwrap(); assert!(remaining.is_empty()); let content = fs::read_to_string(clone_b.path().join("conflict.md")).unwrap(); - assert_eq!(content, "# Version B\n"); + assert_eq!(content, expected_content); + } + + #[test] + fn test_resolve_conflict_ours() { + assert_resolve_conflict_strategy("ours", "# Version B\n"); } #[test] fn test_resolve_conflict_theirs() { - let (_bare, _clone_a, clone_b) = setup_conflict_pair(); - let vp_b = clone_b.path().to_str().unwrap(); - - git_resolve_conflict(vp_b, "conflict.md", "theirs").unwrap(); - - let remaining = get_conflict_files(vp_b).unwrap(); - assert!(remaining.is_empty()); - - let content = fs::read_to_string(clone_b.path().join("conflict.md")).unwrap(); - assert_eq!(content, "# Version A\n"); + assert_resolve_conflict_strategy("theirs", "# Version A\n"); } #[test] @@ -244,7 +237,7 @@ mod tests { let result = git_commit_conflict_resolution(vp_b); assert!(result.is_ok()); - let log = Command::new("git") + let log = git_command() .args(["log", "--oneline", "-1"]) .current_dir(clone_b.path()) .output() @@ -310,7 +303,7 @@ mod tests { fs::write(clone_b_dir.path().join("conflict.md"), "# Version B\n").unwrap(); git_commit(vp_b, "B's change").unwrap(); - let output = Command::new("git") + let output = git_command() .args(["pull", "--rebase"]) .current_dir(clone_b_dir.path()) .output() diff --git a/src-tauri/src/git/connect.rs b/src-tauri/src/git/connect.rs index 7ed8cfc1..354621bb 100644 --- a/src-tauri/src/git/connect.rs +++ b/src-tauri/src/git/connect.rs @@ -1,6 +1,8 @@ use serde::{Deserialize, Serialize}; use std::path::Path; -use std::process::{Command, Output}; +use std::process::Output; + +use super::git_command; const DEFAULT_REMOTE_NAME: &str = "origin"; @@ -192,14 +194,14 @@ fn list_remotes(vault: &Path) -> Result, String> { } fn unset_upstream(vault: &Path) { - let _ = Command::new("git") + let _ = git_command() .args(["branch", "--unset-upstream"]) .current_dir(vault) .output(); } fn run_git(vault: &Path, args: &[&str]) -> Result<(), String> { - let output = Command::new("git") + let output = git_command() .args(args) .current_dir(vault) .output() @@ -237,7 +239,7 @@ fn list_remote_branches(vault: &Path) -> Result, String> { } fn histories_share_base(vault: &Path, connection: &RemoteConnection) -> bool { - Command::new("git") + git_command() .args(["merge-base", "HEAD", connection.remote_branch.as_str()]) .current_dir(vault) .output() @@ -315,7 +317,7 @@ fn classify_connect_error(stderr: &str) -> GitAddRemoteResult { } fn git_output(vault: &Path, args: &[&str]) -> Result { - Command::new("git") + git_command() .args(args) .current_dir(vault) .output() diff --git a/src-tauri/src/git/dates.rs b/src-tauri/src/git/dates.rs index 27a83f7a..8a2303fa 100644 --- a/src-tauri/src/git/dates.rs +++ b/src-tauri/src/git/dates.rs @@ -1,7 +1,7 @@ +use super::git_command; use chrono::DateTime; use std::collections::HashMap; use std::path::Path; -use std::process::Command; /// Git-derived creation and modification timestamps for a file. #[derive(Debug, Clone)] @@ -19,7 +19,7 @@ pub struct GitDates { /// Files not yet committed (untracked / only staged) will not appear in the map; /// callers should fall back to filesystem metadata for those. pub fn get_all_file_dates(vault_path: &Path) -> HashMap { - let output = match Command::new("git") + let output = match git_command() .args(["log", "--format=COMMIT %aI", "--name-only"]) .current_dir(vault_path) .output() diff --git a/src-tauri/src/git/history.rs b/src-tauri/src/git/history.rs index e0483ccb..b9336282 100644 --- a/src-tauri/src/git/history.rs +++ b/src-tauri/src/git/history.rs @@ -1,5 +1,5 @@ +use super::git_command; use std::path::Path; -use std::process::Command; use super::GitCommit; @@ -16,7 +16,7 @@ pub fn get_file_history(vault_path: &str, file_path: &str) -> Result Result Result Result Command { + crate::hidden_command("git") +} + /// Ensure a `.gitignore` with sensible defaults exists in the vault directory. /// Creates the file if missing; leaves existing `.gitignore` files untouched. pub fn ensure_gitignore(path: &str) -> Result<(), String> { @@ -99,7 +103,7 @@ fn commit_initial_vault_setup(dir: &Path) -> Result<(), String> { /// Run a git command in the given directory, returning an error on failure. fn run_git(dir: &Path, args: &[&str]) -> Result<(), String> { - let output = Command::new("git") + let output = git_command() .args(args) .current_dir(dir) .output() @@ -130,7 +134,7 @@ fn ensure_author_config(dir: &Path) -> Result<(), String> { ("user.name", "Tolaria"), ("user.email", "vault@tolaria.app"), ] { - let check = Command::new("git") + let check = git_command() .args(["config", key]) .current_dir(dir) .output() @@ -174,7 +178,6 @@ fn parse_github_repo_path(url: &str) -> Option { mod tests { use super::*; use std::fs; - use std::process::Command; use tempfile::TempDir; fn assert_repo_path(url: &str, expected: Option<&str>) { @@ -188,19 +191,19 @@ mod tests { let dir = TempDir::new().unwrap(); let path = dir.path(); - Command::new("git") + git_command() .args(["init", "--initial-branch=main"]) .current_dir(path) .output() .unwrap(); - Command::new("git") + git_command() .args(["config", "user.email", "test@test.com"]) .current_dir(path) .output() .unwrap(); - Command::new("git") + git_command() .args(["config", "user.name", "Test User"]) .current_dir(path) .output() @@ -214,14 +217,14 @@ mod tests { let bare_dir = TempDir::new().unwrap(); let bare = bare_dir.path(); - Command::new("git") + git_command() .args(["init", "--bare"]) .current_dir(bare) .output() .unwrap(); let clone_a_dir = TempDir::new().unwrap(); - Command::new("git") + git_command() .args(["clone", bare.to_str().unwrap(), "."]) .current_dir(clone_a_dir.path()) .output() @@ -230,7 +233,7 @@ mod tests { &["config", "user.email", "a@test.com"][..], &["config", "user.name", "User A"][..], ] { - Command::new("git") + git_command() .args(*cmd) .current_dir(clone_a_dir.path()) .output() @@ -238,7 +241,7 @@ mod tests { } let clone_b_dir = TempDir::new().unwrap(); - Command::new("git") + git_command() .args(["clone", bare.to_str().unwrap(), "."]) .current_dir(clone_b_dir.path()) .output() @@ -247,7 +250,7 @@ mod tests { &["config", "user.email", "b@test.com"][..], &["config", "user.name", "User B"][..], ] { - Command::new("git") + git_command() .args(*cmd) .current_dir(clone_b_dir.path()) .output() @@ -301,7 +304,7 @@ mod tests { init_repo(vault.to_str().unwrap()).unwrap(); - let log = Command::new("git") + let log = git_command() .args(["log", "--oneline"]) .current_dir(&vault) .output() @@ -317,17 +320,17 @@ mod tests { fs::create_dir_all(&vault).unwrap(); fs::write(vault.join("note.md"), "# Test\n").unwrap(); - Command::new("git") + git_command() .args(["init"]) .current_dir(&vault) .output() .unwrap(); - Command::new("git") + git_command() .args(["config", "commit.gpgsign", "true"]) .current_dir(&vault) .output() .unwrap(); - Command::new("git") + git_command() .args(["config", "gpg.program", "/missing/tolaria-test-gpg"]) .current_dir(&vault) .output() @@ -335,7 +338,7 @@ mod tests { init_repo(vault.to_str().unwrap()).unwrap(); - let log = Command::new("git") + let log = git_command() .args(["log", "--oneline"]) .current_dir(&vault) .output() @@ -353,7 +356,7 @@ mod tests { init_repo(vault.to_str().unwrap()).unwrap(); - let status = Command::new("git") + let status = git_command() .args(["status", "--porcelain"]) .current_dir(&vault) .output() diff --git a/src-tauri/src/git/pulse.rs b/src-tauri/src/git/pulse.rs index fe148e22..13c3fdb7 100644 --- a/src-tauri/src/git/pulse.rs +++ b/src-tauri/src/git/pulse.rs @@ -1,8 +1,7 @@ use serde::Serialize; use std::path::Path; -use std::process::Command; -use super::parse_github_repo_path; +use super::{git_command, parse_github_repo_path}; #[derive(Debug, Serialize, Clone)] pub struct PulseFile { @@ -67,7 +66,7 @@ pub fn get_vault_pulse( let limit_str = limit.to_string(); let skip_str = skip.to_string(); - let output = Command::new("git") + let output = git_command() .args([ "log", "--name-status", @@ -99,7 +98,7 @@ pub fn get_vault_pulse( fn get_github_base_url(vault_path: &str) -> Option { let vault = Path::new(vault_path); - let output = Command::new("git") + let output = git_command() .args(["remote", "get-url", "origin"]) .current_dir(vault) .output() @@ -123,69 +122,90 @@ fn parse_pulse_output(stdout: &str, github_base: &Option) -> Vec 1 && line.as_bytes().get(1) == Some(&b'\t') - }) - { - // Commit header line: hash|short_hash|message|date - if let Some(commit) = current.take() { - commits.push(commit); - } - let parts: Vec<&str> = line.splitn(4, '|').collect(); - if parts.len() == 4 { - let hash = parts[0]; - let date = chrono::DateTime::parse_from_rfc3339(parts[3]) - .map(|dt| dt.timestamp()) - .unwrap_or(0); - let github_url = github_base - .as_ref() - .map(|base| format!("{}/commit/{}", base, hash)); + if is_commit_header(line) { + push_current_commit(&mut commits, &mut current); + current = parse_commit_header(line, github_base); + continue; + } - current = Some(PulseCommit { - hash: hash.to_string(), - short_hash: parts[1].to_string(), - message: parts[2].to_string(), - date, - github_url, - files: Vec::new(), - added: 0, - modified: 0, - deleted: 0, - }); - } - } else if let Some(ref mut commit) = current { - // File status line: A\tpath or M\tpath - let file_parts: Vec<&str> = line.splitn(2, '\t').collect(); - if file_parts.len() == 2 { - let status = parse_file_status(file_parts[0].trim()); - let path = file_parts[1].trim(); - match status { - "added" => commit.added += 1, - "deleted" => commit.deleted += 1, - _ => commit.modified += 1, - } - commit.files.push(PulseFile { - path: path.to_string(), - status: status.to_string(), - title: title_from_path(path), - }); - } + if let Some(ref mut commit) = current { + add_file_change(commit, line); } } - if let Some(commit) = current { - commits.push(commit); - } + push_current_commit(&mut commits, &mut current); commits } +fn is_git_status_line(line: &str) -> bool { + line.starts_with(|c: char| { + c.is_ascii_uppercase() && line.len() > 1 && line.as_bytes().get(1) == Some(&b'\t') + }) +} + +fn is_commit_header(line: &str) -> bool { + line.contains('|') && !is_git_status_line(line) +} + +fn push_current_commit(commits: &mut Vec, current: &mut Option) { + if let Some(commit) = current.take() { + commits.push(commit); + } +} + +fn parse_commit_header(line: &str, github_base: &Option) -> Option { + let parts: Vec<&str> = line.splitn(4, '|').collect(); + if parts.len() != 4 { + return None; + } + + let hash = parts[0]; + let date = chrono::DateTime::parse_from_rfc3339(parts[3]) + .map(|dt| dt.timestamp()) + .unwrap_or(0); + let github_url = github_base + .as_ref() + .map(|base| format!("{}/commit/{}", base, hash)); + + Some(PulseCommit { + hash: hash.to_string(), + short_hash: parts[1].to_string(), + message: parts[2].to_string(), + date, + github_url, + files: Vec::new(), + added: 0, + modified: 0, + deleted: 0, + }) +} + +fn add_file_change(commit: &mut PulseCommit, line: &str) { + let file_parts: Vec<&str> = line.splitn(2, '\t').collect(); + if file_parts.len() != 2 { + return; + } + + let status = parse_file_status(file_parts[0].trim()); + let path = file_parts[1].trim(); + match status { + "added" => commit.added += 1, + "deleted" => commit.deleted += 1, + _ => commit.modified += 1, + } + commit.files.push(PulseFile { + path: path.to_string(), + status: status.to_string(), + title: title_from_path(path), + }); +} + /// Get the last commit's short hash and a GitHub URL (if remote is GitHub). pub fn get_last_commit_info(vault_path: &str) -> Result, String> { let vault = Path::new(vault_path); - let output = Command::new("git") + let output = git_command() .args(["log", "-1", "--format=%H|%h"]) .current_dir(vault) .output() @@ -223,23 +243,7 @@ pub fn get_last_commit_info(vault_path: &str) -> Result, /// Try to build a GitHub commit URL from the origin remote URL. fn get_github_commit_url(vault_path: &str, full_hash: &str) -> Option { - let vault = Path::new(vault_path); - let output = Command::new("git") - .args(["remote", "get-url", "origin"]) - .current_dir(vault) - .output() - .ok()?; - - if !output.status.success() { - return None; - } - - let url = String::from_utf8_lossy(&output.stdout).trim().to_string(); - let repo_path = parse_github_repo_path(&url)?; - Some(format!( - "https://github.com/{}/commit/{}", - repo_path, full_hash - )) + get_github_base_url(vault_path).map(|base| format!("{}/commit/{}", base, full_hash)) } #[cfg(test)] diff --git a/src-tauri/src/git/remote.rs b/src-tauri/src/git/remote.rs index 70401f14..89234787 100644 --- a/src-tauri/src/git/remote.rs +++ b/src-tauri/src/git/remote.rs @@ -1,6 +1,6 @@ +use super::git_command; use serde::{Deserialize, Serialize}; use std::path::Path; -use std::process::Command; use super::conflict::get_conflict_files; @@ -17,7 +17,7 @@ pub struct GitPullResult { /// Check whether the vault repo has at least one remote configured. pub fn has_remote(vault_path: &str) -> Result { let vault = Path::new(vault_path); - let output = Command::new("git") + let output = git_command() .args(["remote"]) .current_dir(vault) .output() @@ -40,7 +40,7 @@ pub fn git_pull(vault_path: &str) -> Result { }); } - let output = Command::new("git") + let output = git_command() .args(["pull", "--no-rebase"]) .current_dir(vault) .output() @@ -134,14 +134,14 @@ pub fn git_remote_status(vault_path: &str) -> Result { } // Fetch latest remote refs (silent, best-effort) - let _ = Command::new("git") + let _ = git_command() .args(["fetch", "--quiet"]) .current_dir(vault) .output(); let branch = current_branch(vault)?; - let output = Command::new("git") + let output = git_command() .args(["rev-list", "--left-right", "--count", "HEAD...@{upstream}"]) .current_dir(vault) .output() @@ -171,7 +171,7 @@ pub fn git_remote_status(vault_path: &str) -> Result { } fn current_branch(vault: &Path) -> Result { - let output = Command::new("git") + let output = git_command() .args(["branch", "--show-current"]) .current_dir(vault) .output() @@ -189,59 +189,93 @@ pub struct GitPushResult { pub fn classify_push_error(stderr: &str) -> GitPushResult { let lower = stderr.to_lowercase(); - if lower.contains("non-fast-forward") - || lower.contains("[rejected]") - || lower.contains("fetch first") - || lower.contains("failed to push some refs") - && (lower.contains("updates were rejected") || lower.contains("non-fast-forward")) - { - return GitPushResult { - status: "rejected".to_string(), - message: "Push rejected: remote has new commits. Pull first, then push.".to_string(), - }; + if is_rejected_push_error(&lower) { + return push_error( + "rejected", + "Push rejected: remote has new commits. Pull first, then push.", + ); } - if lower.contains("authentication failed") - || lower.contains("could not read username") - || lower.contains("permission denied") - || lower.contains("403") - || lower.contains("invalid credentials") - { - return GitPushResult { - status: "auth_error".to_string(), - message: "Push failed: authentication error. Check your credentials.".to_string(), - }; + if is_auth_push_error(&lower) { + return push_error( + "auth_error", + "Push failed: authentication error. Check your credentials.", + ); } - if lower.contains("could not resolve host") - || lower.contains("unable to access") - || lower.contains("connection refused") - || lower.contains("network is unreachable") - || lower.contains("timed out") - { - return GitPushResult { - status: "network_error".to_string(), - message: "Push failed: network error. Check your connection and try again.".to_string(), - }; + if is_network_push_error(&lower) { + return push_error( + "network_error", + "Push failed: network error. Check your connection and try again.", + ); } - // Fallback: extract the hint line if present, otherwise use the full stderr + push_error( + "error", + format!("Push failed: {}", push_error_detail(stderr)), + ) +} + +fn push_error(status: &str, message: impl Into) -> GitPushResult { + GitPushResult { + status: status.to_string(), + message: message.into(), + } +} + +fn contains_any(haystack: &str, needles: &[&str]) -> bool { + needles.iter().any(|needle| haystack.contains(needle)) +} + +fn is_rejected_push_error(lower: &str) -> bool { + contains_any(lower, &["non-fast-forward", "[rejected]", "fetch first"]) + || (lower.contains("failed to push some refs") + && contains_any(lower, &["updates were rejected", "non-fast-forward"])) +} + +fn is_auth_push_error(lower: &str) -> bool { + contains_any( + lower, + &[ + "authentication failed", + "could not read username", + "permission denied", + "403", + "invalid credentials", + ], + ) +} + +fn is_network_push_error(lower: &str) -> bool { + contains_any( + lower, + &[ + "could not resolve host", + "unable to access", + "connection refused", + "network is unreachable", + "timed out", + ], + ) +} + +fn push_error_detail(stderr: &str) -> String { let hint_line = stderr .lines() - .find(|l| l.trim_start().starts_with("hint:")) - .map(|l| l.trim_start().strip_prefix("hint:").unwrap_or(l).trim()) + .find(|line| line.trim_start().starts_with("hint:")) + .map(|line| { + line.trim_start() + .strip_prefix("hint:") + .unwrap_or(line) + .trim() + }) .unwrap_or("") .to_string(); - let detail = if hint_line.is_empty() { + if hint_line.is_empty() { stderr.trim().to_string() } else { hint_line - }; - - GitPushResult { - status: "error".to_string(), - message: format!("Push failed: {detail}"), } } @@ -249,7 +283,7 @@ pub fn classify_push_error(stderr: &str) -> GitPushResult { pub fn git_push(vault_path: &str) -> Result { let vault = Path::new(vault_path); - let output = Command::new("git") + let output = git_command() .args(["push"]) .current_dir(vault) .output() @@ -272,7 +306,6 @@ mod tests { use crate::git::git_commit; use crate::git::tests::{setup_git_repo, setup_remote_pair}; use std::fs; - use std::process::Command; #[test] fn test_has_remote_returns_false_for_local_repo() { @@ -289,7 +322,7 @@ mod tests { let vault = dir.path(); let vp = vault.to_str().unwrap(); - Command::new("git") + git_command() .args(["remote", "add", "origin", "https://example.com/repo.git"]) .current_dir(vault) .output() diff --git a/src-tauri/src/git/status.rs b/src-tauri/src/git/status.rs index 30b44af5..6e340be9 100644 --- a/src-tauri/src/git/status.rs +++ b/src-tauri/src/git/status.rs @@ -1,7 +1,7 @@ +use super::git_command; use serde::Serialize; use std::collections::HashMap; use std::path::Path; -use std::process::Command; #[derive(Debug, Serialize, Clone)] pub struct ModifiedFile { @@ -23,14 +23,34 @@ struct DiffStats { binary: bool, } -fn status_label(status_code: &str) -> &'static str { - match status_code.trim() { - "M" | "MM" | "AM" => "modified", - "A" => "added", - "D" => "deleted", - "??" => "untracked", - "R" | "RM" => "renamed", - _ => "modified", +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum FileChangeStatus { + Modified, + Added, + Deleted, + Untracked, + Renamed, +} + +impl FileChangeStatus { + fn from_code(status_code: &str) -> Self { + match status_code.trim() { + "A" => Self::Added, + "D" => Self::Deleted, + "??" => Self::Untracked, + "R" | "RM" => Self::Renamed, + _ => Self::Modified, + } + } + + fn label(self) -> &'static str { + match self { + Self::Added => "added", + Self::Deleted => "deleted", + Self::Untracked => "untracked", + Self::Renamed => "renamed", + Self::Modified => "modified", + } } } @@ -68,7 +88,7 @@ fn parse_numstat_line(line: &str) -> Option<(String, DiffStats)> { } fn repo_has_head(vault: &Path) -> Result { - let output = Command::new("git") + let output = git_command() .args(["rev-parse", "--verify", "HEAD"]) .current_dir(vault) .output() @@ -82,7 +102,7 @@ fn load_diff_stats(vault: &Path) -> Result, String> { return Ok(HashMap::new()); } - let output = Command::new("git") + let output = git_command() .args(["diff", "--numstat", "--find-renames", "HEAD", "--"]) .current_dir(vault) .output() @@ -100,7 +120,7 @@ fn load_diff_stats(vault: &Path) -> Result, String> { .collect::>()) } -fn count_worktree_lines(vault: &Path, relative_path: &str) -> DiffStats { +fn count_worktree_lines(vault: &Path, relative_path: &Path) -> DiffStats { let full_path = vault.join(relative_path); let added_lines = std::fs::read_to_string(full_path) .ok() @@ -115,19 +135,20 @@ fn count_worktree_lines(vault: &Path, relative_path: &str) -> DiffStats { fn resolve_diff_stats( vault: &Path, - relative_path: &str, - status: &str, + relative_path: &Path, + status: FileChangeStatus, diff_stats: &HashMap, ) -> DiffStats { - if status == "untracked" { + if status == FileChangeStatus::Untracked { return count_worktree_lines(vault, relative_path); } - diff_stats.get(relative_path).copied().unwrap_or_default() + let key = relative_path.to_string_lossy(); + diff_stats.get(key.as_ref()).copied().unwrap_or_default() } -fn ensure_path_within_vault(vault: &Path, relative_path: &str, abs: &Path) -> Result<(), String> { - for component in Path::new(relative_path).components() { +fn ensure_path_within_vault(vault: &Path, relative_path: &Path, abs: &Path) -> Result<(), String> { + for component in relative_path.components() { if matches!(component, std::path::Component::ParentDir) { return Err("File path is outside the vault".into()); } @@ -151,9 +172,10 @@ fn ensure_path_within_vault(vault: &Path, relative_path: &str, abs: &Path) -> Re } } -fn load_file_status(vault: &Path, relative_path: &str) -> Result { - let output = Command::new("git") - .args(["status", "--porcelain", "--", relative_path]) +fn load_file_status(vault: &Path, relative_path: &Path) -> Result { + let output = git_command() + .args(["status", "--porcelain", "--"]) + .arg(relative_path) .current_dir(vault) .output() .map_err(|e| format!("Failed to run git status: {e}"))?; @@ -166,14 +188,16 @@ fn load_file_status(vault: &Path, relative_path: &str) -> Result .unwrap_or_default()) } -fn restore_tracked_file(vault: &Path, relative_path: &str) -> Result<(), String> { - let _ = Command::new("git") - .args(["reset", "HEAD", "--", relative_path]) +fn restore_tracked_file(vault: &Path, relative_path: &Path) -> Result<(), String> { + let _ = git_command() + .args(["reset", "HEAD", "--"]) + .arg(relative_path) .current_dir(vault) .output(); - let checkout = Command::new("git") - .args(["checkout", "--", relative_path]) + let checkout = git_command() + .args(["checkout", "--"]) + .arg(relative_path) .current_dir(vault) .output() .map_err(|e| format!("Failed to run git checkout: {e}"))?; @@ -189,7 +213,7 @@ fn restore_tracked_file(vault: &Path, relative_path: &str) -> Result<(), String> /// Get list of modified/added/deleted files in the vault (uncommitted changes). pub fn get_modified_files(vault_path: &str) -> Result, String> { let vault = Path::new(vault_path); - let output = Command::new("git") + let output = git_command() .args(["status", "--porcelain", "--untracked-files=all"]) .current_dir(vault) .output() @@ -217,14 +241,14 @@ pub fn get_modified_files(vault_path: &str) -> Result, String> return None; } - let status = status_label(status_code); + let status = FileChangeStatus::from_code(status_code); let full_path = vault.join(&relative_path).to_string_lossy().to_string(); - let stats = resolve_diff_stats(vault, &relative_path, status, &diff_stats); + let stats = resolve_diff_stats(vault, Path::new(&relative_path), status, &diff_stats); Some(ModifiedFile { path: full_path, relative_path, - status: status.to_string(), + status: status.label().to_string(), added_lines: stats.added_lines, deleted_lines: stats.deleted_lines, binary: stats.binary, @@ -244,10 +268,11 @@ pub fn get_modified_files(vault_path: &str) -> Result, String> /// returned by [`get_modified_files`]). pub fn discard_file_changes(vault_path: &str, relative_path: &str) -> Result<(), String> { let vault = Path::new(vault_path); - let abs = vault.join(relative_path); + let relative = Path::new(relative_path); + let abs = vault.join(relative); - ensure_path_within_vault(vault, relative_path, &abs)?; - let status_code = load_file_status(vault, relative_path)?; + ensure_path_within_vault(vault, relative, &abs)?; + let status_code = load_file_status(vault, relative)?; match status_code.as_str() { "??" => { @@ -255,7 +280,7 @@ pub fn discard_file_changes(vault_path: &str, relative_path: &str) -> Result<(), .map_err(|e| format!("Failed to delete untracked file: {e}"))?; } _ => { - restore_tracked_file(vault, relative_path)?; + restore_tracked_file(vault, relative)?; } } @@ -264,11 +289,11 @@ pub fn discard_file_changes(vault_path: &str, relative_path: &str) -> Result<(), #[cfg(test)] mod tests { + use super::git_command; use super::*; use crate::git::git_commit; use crate::git::tests::setup_git_repo; use std::fs; - use std::process::Command; fn write_and_commit_markdown(vault: &Path, vp: &str, relative_path: &str, content: &str) { fs::write(vault.join(relative_path), content).unwrap(); @@ -282,12 +307,12 @@ mod tests { // Create and commit a file fs::write(vault.join("note.md"), "# Note\n").unwrap(); - Command::new("git") + git_command() .args(["add", "note.md"]) .current_dir(vault) .output() .unwrap(); - Command::new("git") + git_command() .args(["commit", "-m", "Add note"]) .current_dir(vault) .output() @@ -327,12 +352,12 @@ mod tests { // Create initial commit so git is initialized fs::write(vault.join("init.md"), "# Init\n").unwrap(); - Command::new("git") + git_command() .args(["add", "init.md"]) .current_dir(vault) .output() .unwrap(); - Command::new("git") + git_command() .args(["commit", "-m", "Initial"]) .current_dir(vault) .output() diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 558d3a6b..a0baff49 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -13,6 +13,9 @@ pub mod telemetry; pub mod vault; pub mod vault_list; +use std::ffi::OsStr; +use std::process::Command; + #[cfg(desktop)] use std::path::{Path, PathBuf}; #[cfg(desktop)] @@ -20,6 +23,24 @@ use std::process::Child; #[cfg(desktop)] use std::sync::Mutex; +#[cfg(windows)] +const CREATE_NO_WINDOW: u32 = 0x08000000; + +pub(crate) fn hidden_command(program: impl AsRef) -> Command { + let mut command = Command::new(program); + suppress_windows_console(&mut command); + command +} + +#[cfg(windows)] +fn suppress_windows_console(command: &mut Command) { + use std::os::windows::process::CommandExt; + command.creation_flags(CREATE_NO_WINDOW); +} + +#[cfg(not(windows))] +fn suppress_windows_console(_command: &mut Command) {} + #[cfg(desktop)] struct WsBridgeChild(Mutex>); diff --git a/src-tauri/src/mcp.rs b/src-tauri/src/mcp.rs index 8c173bbb..dae745da 100644 --- a/src-tauri/src/mcp.rs +++ b/src-tauri/src/mcp.rs @@ -17,10 +17,9 @@ pub enum McpStatus { /// Find the `node` binary path at runtime. pub(crate) fn find_node() -> Result { - let output = Command::new("which") - .arg("node") + let output = node_lookup_command() .output() - .map_err(|e| format!("Failed to run `which node`: {e}"))?; + .map_err(|e| format!("Failed to locate node on PATH: {e}"))?; if output.status.success() { let path = String::from_utf8_lossy(&output.stdout).trim().to_string(); if !path.is_empty() { @@ -35,6 +34,16 @@ pub(crate) fn find_node() -> Result { Err("node not found in PATH or common install locations".into()) } +fn node_lookup_command() -> Command { + #[cfg(windows)] + let mut command = crate::hidden_command("where.exe"); + #[cfg(not(windows))] + let mut command = crate::hidden_command("which"); + + command.arg("node"); + command +} + fn fallback_node_path() -> Option { let mut candidates = vec![ PathBuf::from("/opt/homebrew/bin/node"), @@ -99,7 +108,7 @@ pub fn spawn_ws_bridge(vault_path: &str) -> Result { let server_dir = mcp_server_dir()?; let script = server_dir.join("ws-bridge.js"); - let child = Command::new(node) + let child = crate::hidden_command(node) .arg(&script) .env("VAULT_PATH", vault_path) .env("WS_PORT", "9710") diff --git a/src-tauri/src/vault/cache.rs b/src-tauri/src/vault/cache.rs index c2657f04..9086a8e0 100644 --- a/src-tauri/src/vault/cache.rs +++ b/src-tauri/src/vault/cache.rs @@ -125,7 +125,7 @@ fn git_head_hash(vault: &Path) -> Option { /// Run a git command in the given directory and return stdout if successful. fn run_git(vault: &Path, args: &[&str]) -> Option { - let output = std::process::Command::new("git") + let output = crate::hidden_command("git") .args(args) .current_dir(vault) .output() @@ -510,7 +510,7 @@ fn migrate_legacy_cache(vault: &Path) { } // Remove legacy file from git tracking if present - let _ = std::process::Command::new("git") + let _ = crate::hidden_command("git") .args([ "rm", "--cached", @@ -727,17 +727,17 @@ mod tests { } fn init_git_repo(vault: &Path) { - std::process::Command::new("git") + crate::hidden_command("git") .args(["init"]) .current_dir(vault) .output() .unwrap(); - std::process::Command::new("git") + crate::hidden_command("git") .args(["config", "user.email", "test@test.com"]) .current_dir(vault) .output() .unwrap(); - std::process::Command::new("git") + crate::hidden_command("git") .args(["config", "user.name", "Test"]) .current_dir(vault) .output() @@ -756,12 +756,12 @@ mod tests { } fn git_add_commit(vault: &Path, msg: &str) { - std::process::Command::new("git") + crate::hidden_command("git") .args(["add", "."]) .current_dir(vault) .output() .unwrap(); - std::process::Command::new("git") + crate::hidden_command("git") .args(["commit", "-m", msg]) .current_dir(vault) .output() @@ -1121,7 +1121,7 @@ mod tests { // Delete file via filesystem (simulates Finder delete) fs::remove_file(vault.join("remove.md")).unwrap(); // Also stage the deletion so git status is clean for this file - std::process::Command::new("git") + crate::hidden_command("git") .args(["add", "remove.md"]) .current_dir(vault) .output() diff --git a/src-tauri/src/vault/getting_started.rs b/src-tauri/src/vault/getting_started.rs index 06506da1..45fb11f0 100644 --- a/src-tauri/src/vault/getting_started.rs +++ b/src-tauri/src/vault/getting_started.rs @@ -1,6 +1,5 @@ use std::fs; use std::path::{Path, PathBuf}; -use std::process::Command; /// Public starter vault cloned when the user chooses Getting Started. pub const GETTING_STARTED_REPO_URL: &str = @@ -578,7 +577,7 @@ fn refresh_cloned_vault_config_files(vault_path: &Path) -> Result<(), String> { } fn vault_has_pending_changes(vault_path: &Path) -> Result { - let output = Command::new("git") + let output = crate::hidden_command("git") .args(["status", "--porcelain"]) .current_dir(vault_path) .output() @@ -599,7 +598,7 @@ fn ensure_commit_identity(vault_path: &Path) -> Result<(), String> { ("user.name", "Tolaria"), ("user.email", "vault@tolaria.app"), ] { - let output = Command::new("git") + let output = crate::hidden_command("git") .args(["config", key]) .current_dir(vault_path) .output() @@ -609,7 +608,7 @@ fn ensure_commit_identity(vault_path: &Path) -> Result<(), String> { continue; } - let set_output = Command::new("git") + let set_output = crate::hidden_command("git") .args(["config", key, fallback]) .current_dir(vault_path) .output() diff --git a/src-tauri/src/vault/rename.rs b/src-tauri/src/vault/rename.rs index 2ae00ab5..21438264 100644 --- a/src-tauri/src/vault/rename.rs +++ b/src-tauri/src/vault/rename.rs @@ -496,7 +496,7 @@ pub struct DetectedRename { /// Detect renamed files by comparing working tree against HEAD using git diff. pub fn detect_renames(vault: &Path) -> Result, String> { - let output = std::process::Command::new("git") + let output = crate::hidden_command("git") .args(["diff", "HEAD", "--name-status", "--diff-filter=R", "-M"]) .current_dir(vault) .output() diff --git a/src/hooks/useGettingStartedClone.test.ts b/src/hooks/useGettingStartedClone.test.ts index 99f873c7..06267594 100644 --- a/src/hooks/useGettingStartedClone.test.ts +++ b/src/hooks/useGettingStartedClone.test.ts @@ -72,7 +72,7 @@ describe('useGettingStartedClone', () => { }) expect(onSuccess).not.toHaveBeenCalled() - expect(onError).toHaveBeenCalledWith('Could not download Getting Started vault. Check your connection and try again.') + expect(onError).toHaveBeenCalledWith('Could not download Getting Started vault: git clone failed: fatal: unable to access') }) it('surfaces the restart-required message when folder picking is blocked after update install', async () => { diff --git a/src/hooks/useOnboarding.test.ts b/src/hooks/useOnboarding.test.ts index faef1be5..07dffd06 100644 --- a/src/hooks/useOnboarding.test.ts +++ b/src/hooks/useOnboarding.test.ts @@ -262,7 +262,7 @@ describe('useOnboarding', () => { await result.current.handleCreateVault() }) - expect(result.current.error).toBe('Could not download Getting Started vault. Check your connection and try again.') + expect(result.current.error).toBe('Could not download Getting Started vault: git clone failed: fatal: unable to access') expect(result.current.state.status).toBe('welcome') }) diff --git a/src/utils/gettingStartedVault.test.ts b/src/utils/gettingStartedVault.test.ts index e8c59603..4e0a7e6f 100644 --- a/src/utils/gettingStartedVault.test.ts +++ b/src/utils/gettingStartedVault.test.ts @@ -28,8 +28,18 @@ describe('gettingStartedVault', () => { .toBe("Destination '/tmp/Getting Started' already exists and is not empty") }) - it('converts other clone failures into a friendly download message', () => { - expect(formatGettingStartedCloneError('git clone failed: fatal: unable to access')) + it('maps git-not-found clone failures to an installation message', () => { + expect(formatGettingStartedCloneError('Failed to run git clone: The system cannot find the file specified. (os error 2)')) + .toBe('Git is required to download the Getting Started vault. Install Git and try again.') + }) + + it('maps concrete network clone failures to the connection message', () => { + expect(formatGettingStartedCloneError('git clone failed: fatal: unable to access: Could not resolve host: github.com')) .toBe('Could not download Getting Started vault. Check your connection and try again.') }) + + it('preserves unexpected clone failure details', () => { + expect(formatGettingStartedCloneError('git clone failed: fatal: unable to access')) + .toBe('Could not download Getting Started vault: git clone failed: fatal: unable to access') + }) }) diff --git a/src/utils/gettingStartedVault.ts b/src/utils/gettingStartedVault.ts index 66906dc1..6ec87662 100644 --- a/src/utils/gettingStartedVault.ts +++ b/src/utils/gettingStartedVault.ts @@ -7,6 +7,30 @@ const CLONE_PATH_ERRORS = [ 'Target path is required', ] +const GIT_NOT_FOUND_ERRORS = [ + 'no such file or directory', + 'os error 2', + 'program not found', + 'system cannot find the file', +] + +const NETWORK_ERRORS = [ + 'could not resolve host', + 'connection refused', + 'network is unreachable', + 'timed out', + 'failed to connect', + 'ssl connect error', +] + +const AUTH_ERRORS = [ + 'authentication failed', + 'could not read username', + 'permission denied', + 'repository not found', + '403', +] + export function buildGettingStartedVaultPath(parentPath: string): string { const trimmed = parentPath.trim().replace(/[\\/]+$/g, '') if (!trimmed) { @@ -34,5 +58,23 @@ export function formatGettingStartedCloneError(err: unknown): string { return message } - return 'Could not download Getting Started vault. Check your connection and try again.' + const lower = message.toLowerCase() + if (GIT_NOT_FOUND_ERRORS.some(fragment => lower.includes(fragment))) { + return 'Git is required to download the Getting Started vault. Install Git and try again.' + } + if (AUTH_ERRORS.some(fragment => lower.includes(fragment))) { + return 'Could not download Getting Started vault. Check your GitHub access and try again.' + } + if (NETWORK_ERRORS.some(fragment => lower.includes(fragment))) { + return 'Could not download Getting Started vault. Check your connection and try again.' + } + + return `Could not download Getting Started vault: ${firstCloneErrorLine(message)}` +} + +function firstCloneErrorLine(message: string): string { + return message + .split(/\r?\n/) + .map(line => line.trim()) + .find(Boolean) ?? 'git reported an unknown error' } diff --git a/tests/smoke/getting-started-template.spec.ts b/tests/smoke/getting-started-template.spec.ts index 93510f77..2054cbd0 100644 --- a/tests/smoke/getting-started-template.spec.ts +++ b/tests/smoke/getting-started-template.spec.ts @@ -49,7 +49,7 @@ test('Getting Started template shows inline retry on clone failure and opens aft await page.getByTestId('welcome-create-vault').click() await expect(page.getByTestId('welcome-error')).toContainText( - 'Could not download Getting Started vault. Check your connection and try again.', + 'Could not download Getting Started vault: git clone failed: fatal: unable to access', ) await expect(page.getByTestId('welcome-retry-template')).toBeVisible()