From 5c386598611ebbdda2cab4bcbee999d20327bec0 Mon Sep 17 00:00:00 2001 From: Paddo <653385+paddo@users.noreply.github.com> Date: Sat, 4 Apr 2026 14:43:32 +1000 Subject: [PATCH 1/2] refactor: reduce duplication and fix compile errors - Extract sha256_hex() utility to lib.rs, replacing 34 inline calls - Extract App::spawn_sync() and App::reload_state() in dashboard - Consolidate 4 identical confirm popup renders into render_confirm_popup() - Remove redundant App::syncing bool (equivalent to sync_child.is_some()) - Fix known_profile_names() re-reading Config from disk on every render - Add DEFAULT_PROFILE constant, replace scattered "dev" literals - Add missing add_profile_dotfile/remove_profile_dotfile functions - Add installing indicator to dashboard status bar - Fix matches!() without assert!() in secret scanner tests - Add tests for sha256_hex and canonical_project_file_path traversal - Tighten weak test assertions --- src/cli/commands/diff.rs | 5 +- src/cli/commands/init.rs | 7 +- src/cli/commands/resolve.rs | 5 +- src/cli/commands/sync.rs | 54 +++-- src/config.rs | 12 +- src/daemon/server.rs | 17 +- src/dashboard/config_edit.rs | 33 +++ src/dashboard/mod.rs | 324 ++++++++---------------------- src/dashboard/widgets/files.rs | 2 +- src/dashboard/widgets/machines.rs | 2 +- src/dashboard/widgets/status.rs | 11 +- src/lib.rs | 19 ++ src/packages/manager.rs | 3 +- src/security/secrets.rs | 8 +- src/sync/conflict.rs | 9 +- src/sync/git.rs | 4 +- src/sync/mod.rs | 25 ++- src/sync/packages.rs | 11 +- 18 files changed, 237 insertions(+), 314 deletions(-) diff --git a/src/cli/commands/diff.rs b/src/cli/commands/diff.rs index f1f956e..e924c83 100644 --- a/src/cli/commands/diff.rs +++ b/src/cli/commands/diff.rs @@ -4,7 +4,6 @@ use crate::sync::{GitBackend, MachineState, SyncEngine, SyncState}; use anyhow::Result; use comfy_table::{Attribute, Cell, Color}; use owo_colors::OwoColorize; -use sha2::{Digest, Sha256}; use std::collections::{HashMap, HashSet}; pub async fn run(machine: Option<&str>) -> Result<()> { @@ -103,7 +102,7 @@ fn show_dotfile_diff( (true, true) => { // Both exist - check if different let local_content = std::fs::read(&local_path)?; - let local_hash = format!("{:x}", Sha256::digest(&local_content)); + let local_hash = crate::sha256_hex(&local_content); let is_different = state .files @@ -389,7 +388,7 @@ fn build_current_machine_state( let path = home.join(file); if path.exists() { let content = std::fs::read(&path)?; - let hash = format!("{:x}", sha2::Sha256::digest(&content)); + let hash = crate::sha256_hex(&content); machine.files.insert(file.to_string(), hash); } } diff --git a/src/cli/commands/init.rs b/src/cli/commands/init.rs index 6979f09..e903fee 100644 --- a/src/cli/commands/init.rs +++ b/src/cli/commands/init.rs @@ -228,9 +228,10 @@ fn assign_profile_during_init(config: &mut Config) -> Result<()> { // No profiles exist yet — v1->v2 migration should have created "dev" // but if it hasn't (e.g., fresh init), create it now config.migrate_v1_to_v2(); - config - .machine_profiles - .insert(machine_id.clone(), "dev".to_string()); + config.machine_profiles.insert( + machine_id.clone(), + crate::config::DEFAULT_PROFILE.to_string(), + ); return Ok(()); } diff --git a/src/cli/commands/resolve.rs b/src/cli/commands/resolve.rs index 060b729..fff3e31 100644 --- a/src/cli/commands/resolve.rs +++ b/src/cli/commands/resolve.rs @@ -3,7 +3,6 @@ use crate::config::Config; use crate::sync::{ConflictResolution, ConflictState, FileConflict, SyncEngine}; use anyhow::Result; use owo_colors::OwoColorize; -use sha2::{Digest, Sha256}; pub async fn run(file: Option<&str>) -> Result<()> { let config = Config::load()?; @@ -94,9 +93,9 @@ pub async fn run(file: Option<&str>) -> Result<()> { let conflict = FileConflict { file_path: pending.file_path.clone(), - local_hash: format!("{:x}", Sha256::digest(&local_content)), + local_hash: crate::sha256_hex(&local_content), last_synced_hash: None, - remote_hash: format!("{:x}", Sha256::digest(&remote_content)), + remote_hash: crate::sha256_hex(&remote_content), local_content, remote_content, }; diff --git a/src/cli/commands/sync.rs b/src/cli/commands/sync.rs index f84265c..1db390c 100644 --- a/src/cli/commands/sync.rs +++ b/src/cli/commands/sync.rs @@ -8,7 +8,6 @@ use crate::sync::{ import_packages, sync_packages, GitBackend, MachineState, SyncEngine, SyncState, }; use anyhow::Result; -use sha2::{Digest, Sha256}; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; @@ -103,11 +102,12 @@ pub async fn run(dry_run: bool, _force: bool, rediscover: bool) -> Result<()> { let mut state = SyncState::load()?; - // Auto-assign machine to "dev" profile on first run after v2 migration + // Auto-assign machine to default profile on first run after v2 migration if !config.profiles.is_empty() && !config.machine_profiles.contains_key(&state.machine_id) { - config - .machine_profiles - .insert(state.machine_id.clone(), "dev".to_string()); + config.machine_profiles.insert( + state.machine_id.clone(), + crate::config::DEFAULT_PROFILE.to_string(), + ); config.save()?; } @@ -184,7 +184,7 @@ pub async fn run(dry_run: bool, _force: bool, rediscover: bool) -> Result<()> { if source.exists() { if let Ok(content) = std::fs::read(&source) { - let hash = format!("{:x}", Sha256::digest(&content)); + let hash = crate::sha256_hex(&content); let file_changed = state .files @@ -535,7 +535,7 @@ pub fn sync_collab_secrets(config: &Config, home: &Path, state: &mut SyncState) let state_key = format!("collab-secret:{}/{}/{}", collab_name, project_url, filename); let last_synced_hash = state.files.get(&state_key).map(|f| f.hash.as_str()); - let remote_hash = format!("{:x}", Sha256::digest(&decrypted)); + let remote_hash = crate::sha256_hex(&decrypted); // Write to all checkouts of this project for local_project in checkouts { @@ -570,7 +570,7 @@ pub fn sync_collab_secrets(config: &Config, home: &Path, state: &mut SyncState) let should_write = if dest.exists() { let existing = std::fs::read(&dest).unwrap_or_default(); - let local_hash = format!("{:x}", Sha256::digest(&existing)); + let local_hash = crate::sha256_hex(&existing); if local_hash == remote_hash { false // Already in sync } else { @@ -804,10 +804,10 @@ pub fn decrypt_from_repo( } } else { // No true conflict - but preserve local-only changes - let remote_hash = format!("{:x}", Sha256::digest(&plaintext)); + let remote_hash = crate::sha256_hex(&plaintext); let local_hash = std::fs::read(&local_file) .ok() - .map(|c| format!("{:x}", Sha256::digest(&c))); + .map(|c| crate::sha256_hex(&c)); // Only write if local unchanged from last sync AND remote differs let local_unchanged = local_hash.as_deref() == last_synced_hash; @@ -895,10 +895,10 @@ pub fn decrypt_from_repo( let state_key = format!("~/{}", rel_path_no_enc); let last_synced_hash = state.files.get(&state_key).map(|f| f.hash.as_str()); - let remote_hash = format!("{:x}", Sha256::digest(&plaintext)); + let remote_hash = crate::sha256_hex(&plaintext); let local_hash = std::fs::read(&local_file) .ok() - .map(|c| format!("{:x}", Sha256::digest(&c))); + .map(|c| crate::sha256_hex(&c)); let local_unchanged = local_hash.as_deref() == last_synced_hash; if local_unchanged && local_hash.as_ref() != Some(&remote_hash) { write_decrypted(&local_file, &plaintext)?; @@ -1259,7 +1259,7 @@ fn decrypt_project_configs( if let Ok(encrypted_content) = std::fs::read(enc_file) { match crate::security::decrypt(&encrypted_content, key) { Ok(plaintext) => { - let remote_hash = format!("{:x}", Sha256::digest(&plaintext)); + let remote_hash = crate::sha256_hex(&plaintext); let state_key = format!("project:{}/{}", project_name, rel_path_no_enc); let canonical_path = crate::sync::canonical_project_file_path( project_name, @@ -1275,8 +1275,7 @@ fn decrypt_project_configs( let local_file = local_repo_path.join(rel_path_no_enc); // Read actual content (follows symlinks) if let Ok(local_content) = std::fs::read(&local_file) { - let local_hash = - format!("{:x}", Sha256::digest(&local_content)); + let local_hash = crate::sha256_hex(&local_content); if Some(&local_hash) != last_synced_hash.as_ref() && local_hash != remote_hash { @@ -1294,9 +1293,8 @@ fn decrypt_project_configs( } else { // Write decrypted content to canonical location let canonical_content = std::fs::read(&canonical_path).ok(); - let canonical_hash = canonical_content - .as_ref() - .map(|c| format!("{:x}", Sha256::digest(c))); + let canonical_hash = + canonical_content.as_ref().map(|c| crate::sha256_hex(c)); if canonical_hash.as_ref() != Some(&remote_hash) { // Backup canonical file if it exists and differs @@ -1381,10 +1379,8 @@ pub fn sync_tether_config(sync_path: &Path, home: &Path) -> Result). Old ProfilePackagesConfig removed. /// Migration: creates "dev" profile from global dotfiles/dirs/packages. pub const CURRENT_CONFIG_VERSION: u32 = 2; +pub const DEFAULT_PROFILE: &str = "dev"; fn default_config_version() -> u32 { 1 @@ -694,7 +695,7 @@ impl Config { self.machine_profiles .get(machine_id) .map(|s| s.as_str()) - .unwrap_or("dev") + .unwrap_or(DEFAULT_PROFILE) } /// Get the profile assigned to a machine, if any @@ -910,9 +911,10 @@ impl Config { packages, }; - self.profiles.insert("dev".to_string(), dev_profile); + self.profiles + .insert(DEFAULT_PROFILE.to_string(), dev_profile); - // Assign all unassigned machines to "dev" + // Assign all unassigned machines to default profile // (machines already in machine_profiles keep their existing assignment) } @@ -1533,7 +1535,7 @@ files = [".zshrc"] #[test] fn test_profile_name_defaults_to_dev() { let config = Config::default(); - assert_eq!(config.profile_name("any-machine"), "dev"); + assert_eq!(config.profile_name("any-machine"), DEFAULT_PROFILE); } #[test] @@ -1543,7 +1545,7 @@ files = [".zshrc"] .machine_profiles .insert("my-server".to_string(), "server".to_string()); assert_eq!(config.profile_name("my-server"), "server"); - assert_eq!(config.profile_name("other"), "dev"); + assert_eq!(config.profile_name("other"), DEFAULT_PROFILE); } #[test] diff --git a/src/daemon/server.rs b/src/daemon/server.rs index dfe9cb5..0e207cb 100644 --- a/src/daemon/server.rs +++ b/src/daemon/server.rs @@ -7,7 +7,6 @@ use crate::sync::{ }; use anyhow::Result; use chrono::Local; -use sha2::{Digest, Sha256}; use std::path::PathBuf; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, SystemTime}; @@ -228,11 +227,12 @@ impl DaemonServer { // Load state and machine state let mut state = SyncState::load()?; - // Auto-assign machine to "dev" profile on first run after v2 migration + // Auto-assign machine to default profile on first run after v2 migration if !config.profiles.is_empty() && !config.machine_profiles.contains_key(&state.machine_id) { - config - .machine_profiles - .insert(state.machine_id.clone(), "dev".to_string()); + config.machine_profiles.insert( + state.machine_id.clone(), + crate::config::DEFAULT_PROFILE.to_string(), + ); let _ = config.save(); } @@ -279,7 +279,7 @@ impl DaemonServer { let source = home.join(&file); if source.exists() { if let Ok(content) = std::fs::read(&source) { - let hash = format!("{:x}", Sha256::digest(&content)); + let hash = crate::sha256_hex(&content); let file_changed = state .files .get(&file) @@ -405,10 +405,7 @@ impl DaemonServer { state.deferred_casks.sort(); // Only notify if list changed (avoid repeated notifications) - let hash = format!( - "{:x}", - Sha256::digest(state.deferred_casks.join(",").as_bytes()) - ); + let hash = crate::sha256_hex(state.deferred_casks.join(",").as_bytes()); if state.deferred_casks_hash.as_ref() != Some(&hash) { notify_deferred_casks(&state.deferred_casks).ok(); state.deferred_casks_hash = Some(hash); diff --git a/src/dashboard/config_edit.rs b/src/dashboard/config_edit.rs index cb0085b..6294b89 100644 --- a/src/dashboard/config_edit.rs +++ b/src/dashboard/config_edit.rs @@ -431,6 +431,39 @@ pub fn toggle_profile_dotfile_shared(config: &mut Config, machine_id: &str, path config.save().is_ok() } +/// Add a dotfile to the machine's profile. Returns false on duplicate or save failure. +pub fn add_profile_dotfile(config: &mut Config, machine_id: &str, path: &str) -> bool { + use crate::config::ProfileDotfileEntry; + + let profile_name = config.profile_name(machine_id).to_string(); + let profile = match config.profiles.get_mut(&profile_name) { + Some(p) => p, + None => return false, + }; + if profile.dotfiles.iter().any(|e| e.path() == path) { + return false; + } + profile + .dotfiles + .push(ProfileDotfileEntry::Simple(path.to_string())); + config.save().is_ok() +} + +/// Remove a dotfile from the machine's profile by path. Returns false if not found or save failure. +pub fn remove_profile_dotfile(config: &mut Config, machine_id: &str, path: &str) -> bool { + let profile_name = config.profile_name(machine_id).to_string(); + let profile = match config.profiles.get_mut(&profile_name) { + Some(p) => p, + None => return false, + }; + let before = profile.dotfiles.len(); + profile.dotfiles.retain(|e| e.path() != path); + if profile.dotfiles.len() == before { + return false; + } + config.save().is_ok() +} + /// Validate interval format: number followed by s/m/h (e.g. "5m", "30s", "1h") fn is_valid_interval(val: &str) -> bool { if val.len() < 2 { diff --git a/src/dashboard/mod.rs b/src/dashboard/mod.rs index 554f130..b199e7e 100644 --- a/src/dashboard/mod.rs +++ b/src/dashboard/mod.rs @@ -117,7 +117,6 @@ pub struct App { active_tab: Tab, scroll_offsets: [usize; 5], should_quit: bool, - syncing: bool, sync_child: Option, daemon_child: Option, daemon_op: DaemonOp, @@ -128,29 +127,23 @@ pub struct App { flash_error: Option<(Instant, String)>, flash_message: Option<(Instant, String)>, list_edit: Option, - // Packages tab state pkg_expanded: Option, pkg_cursor: usize, uninstall_confirm: Option<(String, String)>, uninstalling: Option<(String, String)>, uninstall_rx: Option>>, - // Machines tab state machine_expanded: Option, machine_cursor: usize, - // Profile picker state profile_editing: bool, - profile_picker_options: Vec, // ["(none)", "dev", "server", ...] + profile_picker_options: Vec, profile_picker_cursor: usize, - // Files tab state files: FilesTabState, file_delete_confirm: Option, file_import_picker: Option, - // Package import state pkg_import_picker: Option, pkg_install_confirm: Option<(String, String)>, installing: Option<(String, String)>, install_rx: Option>>, - // Background package refresh on startup pkg_refresh_rx: Option>>>, } @@ -171,6 +164,28 @@ impl App { &mut self.scroll_offsets[idx] } + fn spawn_sync(&mut self) { + if self.sync_child.is_some() { + return; + } + let exe = std::env::current_exe().unwrap_or_else(|_| "tether".into()); + if let Ok(child) = std::process::Command::new(exe) + .arg("sync") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + { + self.sync_child = Some(child); + } + } + + fn reload_state(&mut self) { + self.state = DashboardState::load(); + self.files.deleted = load_deleted_files(&self.state); + refresh_files_expanded(self); + self.last_refresh = Instant::now(); + } + fn item_count(&self) -> usize { match self.active_tab { Tab::Files => widgets::files::build_rows(&self.state, &self.files).len(), @@ -209,7 +224,6 @@ pub fn run() -> Result<()> { active_tab: Tab::Overview, scroll_offsets: [0; 5], should_quit: false, - syncing: false, sync_child: None, daemon_child: None, daemon_op: DaemonOp::None, @@ -285,48 +299,26 @@ pub fn run() -> Result<()> { } } - // Check sync child process if let Some(ref mut child) = app.sync_child { if let Ok(Some(_)) = child.try_wait() { - app.syncing = false; app.sync_child = None; - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(&mut app); - app.last_refresh = Instant::now(); + app.reload_state(); } } - // Check daemon child process if let Some(ref mut child) = app.daemon_child { if let Ok(Some(_)) = child.try_wait() { app.daemon_op = DaemonOp::None; app.daemon_child = None; - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(&mut app); - app.last_refresh = Instant::now(); + app.reload_state(); } } - // Check uninstall result if let Some(ref rx) = app.uninstall_rx { if let Ok(result) = rx.try_recv() { match result { Ok(()) => { - // Trigger a sync so machine state reflects the removal - if !app.syncing { - let exe = std::env::current_exe().unwrap_or_else(|_| "tether".into()); - if let Ok(child) = std::process::Command::new(exe) - .arg("sync") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - { - app.syncing = true; - app.sync_child = Some(child); - } - } + app.spawn_sync(); } Err(msg) => { app.flash_error = @@ -338,7 +330,6 @@ pub fn run() -> Result<()> { } } - // Check install result if let Some(ref rx) = app.install_rx { if let Ok(result) = rx.try_recv() { match result { @@ -360,19 +351,7 @@ pub fn run() -> Result<()> { } } } - // Trigger sync - if !app.syncing { - let exe = std::env::current_exe().unwrap_or_else(|_| "tether".into()); - if let Ok(child) = std::process::Command::new(exe) - .arg("sync") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - { - app.syncing = true; - app.sync_child = Some(child); - } - } + app.spawn_sync(); } Err(msg) => { app.flash_error = @@ -429,10 +408,7 @@ pub fn run() -> Result<()> { } if app.last_refresh.elapsed() >= refresh_interval { - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(&mut app); - app.last_refresh = Instant::now(); + app.reload_state(); } if app.should_quit { @@ -503,20 +479,7 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { Instant::now(), format!("Restored {} to {}", dotfile_path, short_hash), )); - // Trigger sync - if !app.syncing { - let exe = - std::env::current_exe().unwrap_or_else(|_| "tether".into()); - if let Ok(child) = std::process::Command::new(exe) - .arg("sync") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - { - app.syncing = true; - app.sync_child = Some(child); - } - } + app.spawn_sync(); } Err(e) => { app.flash_error = @@ -544,29 +507,13 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { let ok = config_edit::remove_profile_dotfile(config, &ss.machine_id, &path); if ok { app.flash_message = Some((Instant::now(), format!("removed {}", path))); - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(app); - app.last_refresh = Instant::now(); + app.reload_state(); // Clamp cursor let new_rows = widgets::files::build_rows(&app.state, &app.files); if app.files.cursor >= new_rows.len() { app.files.cursor = new_rows.len().saturating_sub(1); } - // Trigger sync - if !app.syncing { - let exe = - std::env::current_exe().unwrap_or_else(|_| "tether".into()); - if let Ok(child) = std::process::Command::new(exe) - .arg("sync") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - { - app.syncing = true; - app.sync_child = Some(child); - } - } + app.spawn_sync(); } else { app.flash_error = Some((Instant::now(), "remove failed".into())); } @@ -612,10 +559,7 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { } app.flash_message = Some((Instant::now(), format!("imported {}", item.path))); - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(app); - app.last_refresh = Instant::now(); + app.reload_state(); } else { app.flash_error = Some((Instant::now(), "import failed".into())); } @@ -720,10 +664,7 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { if config.save().is_err() { app.flash_error = Some((Instant::now(), "save failed".into())); } - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(app); - app.last_refresh = Instant::now(); + app.reload_state(); } } } @@ -1031,7 +972,11 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { .expanded_file .as_ref() .and_then(|repo_path| { - let dotfile = repo_path_to_dotfile(repo_path, encrypted); + let dotfile = repo_path_to_dotfile( + repo_path, + encrypted, + app.state.config.as_ref(), + ); crate::sync::SyncEngine::sync_path() .ok() .and_then(|p| crate::sync::GitBackend::open(&p).ok()) @@ -1101,18 +1046,7 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { } } KeyCode::Char('s') => { - if !app.syncing { - let exe = std::env::current_exe().unwrap_or_else(|_| "tether".into()); - if let Ok(child) = std::process::Command::new(exe) - .arg("sync") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - { - app.syncing = true; - app.sync_child = Some(child); - } - } + app.spawn_sync(); } KeyCode::Char('d') => { if app.daemon_op == DaemonOp::None && app.daemon_child.is_none() { @@ -1138,10 +1072,7 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { } } KeyCode::Char('r') => { - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(app); - app.last_refresh = Instant::now(); + app.reload_state(); } KeyCode::Char('t') => { if app.active_tab == Tab::Files { @@ -1170,10 +1101,7 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { } ), )); - app.state = DashboardState::load(); - app.files.deleted = load_deleted_files(&app.state); - refresh_files_expanded(app); - app.last_refresh = Instant::now(); + app.reload_state(); } } } @@ -1197,7 +1125,11 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { .as_ref() .map(|c| c.security.encrypt_dotfiles) .unwrap_or(false); - let dotfile = repo_path_to_dotfile(repo_path, encrypted); + let dotfile = repo_path_to_dotfile( + repo_path, + encrypted, + app.state.config.as_ref(), + ); app.files.restore_confirm = Some((dotfile, commit_hash.clone(), short_hash.clone())); } @@ -1607,44 +1539,8 @@ fn run_restore( Ok(()) } -fn render_restore_popup(f: &mut Frame, dotfile_path: &str, short_hash: &str) { - let area = f.area(); - let msg = format!("Restore {} to {}?", dotfile_path, short_hash); - let width = (msg.len() as u16 + 8).min(area.width.saturating_sub(4)); - let height = 5u16.min(area.height.saturating_sub(2)); - let x = (area.width.saturating_sub(width)) / 2; - let y = (area.height.saturating_sub(height)) / 2; - let popup_area = Rect::new(x, y, width, height); - - f.render_widget(ratatui::widgets::Clear, popup_area); - - let text = vec![ - Line::from(""), - Line::from(Span::styled( - format!(" {}", msg), - Style::default().fg(Color::White), - )), - Line::from(""), - Line::from(vec![ - Span::styled(" y", Style::default().fg(Color::Yellow).bold()), - Span::styled(" confirm ", Style::default().fg(Color::DarkGray)), - Span::styled("n/Esc", Style::default().fg(Color::Yellow).bold()), - Span::styled(" cancel", Style::default().fg(Color::DarkGray)), - ]), - ]; - - let paragraph = ratatui::widgets::Paragraph::new(text).block( - ratatui::widgets::Block::default() - .title(" Restore ") - .borders(ratatui::widgets::Borders::ALL) - .border_style(Style::default().fg(Color::Yellow)), - ); - f.render_widget(paragraph, popup_area); -} - -fn render_file_delete_popup(f: &mut Frame, path: &str) { +fn render_confirm_popup(f: &mut Frame, title: &str, msg: &str, border_color: Color) { let area = f.area(); - let msg = format!("Remove {} from profile?", path); let width = (msg.len() as u16 + 8).min(area.width.saturating_sub(4)); let height = 5u16.min(area.height.saturating_sub(2)); let x = (area.width.saturating_sub(width)) / 2; @@ -1670,9 +1566,9 @@ fn render_file_delete_popup(f: &mut Frame, path: &str) { let paragraph = ratatui::widgets::Paragraph::new(text).block( ratatui::widgets::Block::default() - .title(" Remove ") + .title(format!(" {} ", title)) .borders(ratatui::widgets::Borders::ALL) - .border_style(Style::default().fg(Color::Red)), + .border_style(Style::default().fg(border_color)), ); f.render_widget(paragraph, popup_area); } @@ -1823,42 +1719,6 @@ fn render_pkg_import_popup(f: &mut Frame, picker: &PkgImportPickerState) { f.render_widget(paragraph, popup_area); } -fn render_install_popup(f: &mut Frame, manager_key: &str, pkg_name: &str) { - let area = f.area(); - let label = widgets::manager_label(manager_key); - let msg = format!("Install {} ({})?", pkg_name, label); - let width = (msg.len() as u16 + 8).min(area.width.saturating_sub(4)); - let height = 5u16.min(area.height.saturating_sub(2)); - let x = (area.width.saturating_sub(width)) / 2; - let y = (area.height.saturating_sub(height)) / 2; - let popup_area = Rect::new(x, y, width, height); - - f.render_widget(ratatui::widgets::Clear, popup_area); - - let text = vec![ - Line::from(""), - Line::from(Span::styled( - format!(" {}", msg), - Style::default().fg(Color::White), - )), - Line::from(""), - Line::from(vec![ - Span::styled(" y", Style::default().fg(Color::Yellow).bold()), - Span::styled(" confirm ", Style::default().fg(Color::DarkGray)), - Span::styled("n/Esc", Style::default().fg(Color::Yellow).bold()), - Span::styled(" cancel", Style::default().fg(Color::DarkGray)), - ]), - ]; - - let paragraph = ratatui::widgets::Paragraph::new(text).block( - ratatui::widgets::Block::default() - .title(" Install ") - .borders(ratatui::widgets::Borders::ALL) - .border_style(Style::default().fg(Color::Green)), - ); - f.render_widget(paragraph, popup_area); -} - fn draw(f: &mut Frame, app: &App) { let main_chunks = Layout::vertical([ Constraint::Length(3), @@ -1880,7 +1740,7 @@ fn draw(f: &mut Frame, app: &App) { f, main_chunks[0], &app.state, - app.syncing, + app.sync_child.is_some(), app.daemon_op, flash, app.uninstalling.as_ref(), @@ -1967,17 +1827,33 @@ fn draw(f: &mut Frame, app: &App) { // Uninstall confirmation popup if let Some((ref manager_key, ref pkg_name)) = app.uninstall_confirm { - render_uninstall_popup(f, manager_key, pkg_name); + let label = widgets::manager_label(manager_key); + render_confirm_popup( + f, + "Uninstall", + &format!("Uninstall {} ({})?", pkg_name, label), + Color::Red, + ); } // Restore confirmation popup if let Some((ref dotfile_path, _, ref short_hash)) = app.files.restore_confirm { - render_restore_popup(f, dotfile_path, short_hash); + render_confirm_popup( + f, + "Restore", + &format!("Restore {} to {}?", dotfile_path, short_hash), + Color::Yellow, + ); } // File delete confirmation popup if let Some(ref path) = app.file_delete_confirm { - render_file_delete_popup(f, path); + render_confirm_popup( + f, + "Remove", + &format!("Remove {} from profile?", path), + Color::Red, + ); } // File import picker popup @@ -1987,7 +1863,13 @@ fn draw(f: &mut Frame, app: &App) { // Package install confirmation popup if let Some((ref manager_key, ref pkg_name)) = app.pkg_install_confirm { - render_install_popup(f, manager_key, pkg_name); + let label = widgets::manager_label(manager_key); + render_confirm_popup( + f, + "Install", + &format!("Install {} ({})?", pkg_name, label), + Color::Green, + ); } // Package import picker popup @@ -1996,42 +1878,6 @@ fn draw(f: &mut Frame, app: &App) { } } -fn render_uninstall_popup(f: &mut Frame, manager_key: &str, pkg_name: &str) { - let area = f.area(); - let label = widgets::manager_label(manager_key); - let msg = format!("Uninstall {} ({})?", pkg_name, label); - let width = (msg.len() as u16 + 8).min(area.width.saturating_sub(4)); - let height = 5u16.min(area.height.saturating_sub(2)); - let x = (area.width.saturating_sub(width)) / 2; - let y = (area.height.saturating_sub(height)) / 2; - let popup_area = Rect::new(x, y, width, height); - - f.render_widget(ratatui::widgets::Clear, popup_area); - - let text = vec![ - Line::from(""), - Line::from(Span::styled( - format!(" {}", msg), - Style::default().fg(Color::White), - )), - Line::from(""), - Line::from(vec![ - Span::styled(" y", Style::default().fg(Color::Yellow).bold()), - Span::styled(" confirm ", Style::default().fg(Color::DarkGray)), - Span::styled("n/Esc", Style::default().fg(Color::Yellow).bold()), - Span::styled(" cancel", Style::default().fg(Color::DarkGray)), - ]), - ]; - - let paragraph = ratatui::widgets::Paragraph::new(text).block( - ratatui::widgets::Block::default() - .title(" Uninstall ") - .borders(ratatui::widgets::Borders::ALL) - .border_style(Style::default().fg(Color::Red)), - ); - f.render_widget(paragraph, popup_area); -} - fn render_profile_popup(f: &mut Frame, options: &[String], cursor: usize) { let area = f.area(); let title = " Profile (this machine) "; @@ -2119,7 +1965,7 @@ fn load_deleted_files(state: &DashboardState) -> HashMap> { let config_ref = state.config.as_ref(); let profile = config_ref .map(|c| c.profile_name(machine_id)) - .unwrap_or("dev"); + .unwrap_or(crate::config::DEFAULT_PROFILE); let mut state_repo_paths: HashSet = HashSet::new(); for path in ss.files.keys() { @@ -2154,7 +2000,7 @@ fn load_deleted_files(state: &DashboardState) -> HashMap> { continue; } // Reverse map: repo path -> display path - let display = repo_path_to_dotfile(repo_file, encrypted); + let display = repo_path_to_dotfile(repo_file, encrypted, state.config.as_ref()); deleted .entry("Personal".to_string()) .or_default() @@ -2172,19 +2018,19 @@ fn load_deleted_files(state: &DashboardState) -> HashMap> { /// Reverse of dotfile_to_repo_path: "dotfiles/zshrc.enc" -> ".zshrc" /// Also handles profiled paths: "profiles/dev/zshrc.enc" -> ".zshrc" /// Uses known profile names from config to distinguish profile dirs from dotfile subdirs. -fn repo_path_to_dotfile(repo_path: &str, encrypted: bool) -> String { - repo_path_to_dotfile_with_profiles(repo_path, encrypted, &known_profile_names()) -} - -fn known_profile_names() -> HashSet { +fn repo_path_to_dotfile( + repo_path: &str, + encrypted: bool, + config: Option<&crate::Config>, +) -> String { let mut names = HashSet::new(); names.insert("shared".to_string()); - if let Ok(config) = crate::config::Config::load() { + if let Some(config) = config { for name in config.profiles.keys() { names.insert(name.clone()); } } - names + repo_path_to_dotfile_with_profiles(repo_path, encrypted, &names) } fn repo_path_to_dotfile_with_profiles( diff --git a/src/dashboard/widgets/files.rs b/src/dashboard/widgets/files.rs index a60d2d1..48d1f27 100644 --- a/src/dashboard/widgets/files.rs +++ b/src/dashboard/widgets/files.rs @@ -162,7 +162,7 @@ fn collect_sections(state: &DashboardState) -> Vec { let config_ref = state.config.as_ref(); let profile = config_ref .map(|c| c.profile_name(machine_id)) - .unwrap_or("dev"); + .unwrap_or(crate::config::DEFAULT_PROFILE); let shared = config_ref .map(|c| c.is_dotfile_shared(machine_id, path)) .unwrap_or(false); diff --git a/src/dashboard/widgets/machines.rs b/src/dashboard/widgets/machines.rs index 18cfef1..96350d6 100644 --- a/src/dashboard/widgets/machines.rs +++ b/src/dashboard/widgets/machines.rs @@ -48,7 +48,7 @@ pub fn build_rows(state: &DashboardState, expanded: Option<&str>) -> Vec { Success(&'a str), } +#[allow(clippy::too_many_arguments)] pub fn render( f: &mut Frame, area: Rect, @@ -16,6 +17,7 @@ pub fn render( daemon_op: DaemonOp, flash: Option, uninstalling: Option<&(String, String)>, + installing: Option<&(String, String)>, ) { let mut spans = vec![Span::styled( " Tether ", @@ -87,7 +89,6 @@ pub fn render( )); } - // Uninstalling if let Some((_, pkg_name)) = uninstalling { spans.push(Span::raw(" ")); spans.push(Span::styled( @@ -96,6 +97,14 @@ pub fn render( )); } + if let Some((_, pkg_name)) = installing { + spans.push(Span::raw(" ")); + spans.push(Span::styled( + format!("installing {}...", pkg_name), + Style::default().fg(Color::Yellow), + )); + } + // Flash message if let Some(flash_msg) = flash { let (msg, color) = match flash_msg { diff --git a/src/lib.rs b/src/lib.rs index 4f4bde7..e302b3b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,3 +12,22 @@ pub use config::Config; pub fn home_dir() -> anyhow::Result { home::home_dir().ok_or_else(|| anyhow::anyhow!("Could not find home directory")) } + +pub fn sha256_hex(data: &[u8]) -> String { + use sha2::Digest; + format!("{:x}", sha2::Sha256::digest(data)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sha256_hex_known_value() { + // SHA-256("abc") is a well-known test vector + assert_eq!( + sha256_hex(b"abc"), + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + ); + } +} diff --git a/src/packages/manager.rs b/src/packages/manager.rs index 906c067..6eb5d90 100644 --- a/src/packages/manager.rs +++ b/src/packages/manager.rs @@ -99,8 +99,7 @@ pub trait PackageManager: Send + Sync { /// Compute a hash of the current manifest for change detection async fn compute_manifest_hash(&self) -> Result { let manifest = self.export_manifest().await?; - use sha2::{Digest, Sha256}; - Ok(format!("{:x}", Sha256::digest(manifest.as_bytes()))) + Ok(crate::sha256_hex(manifest.as_bytes())) } /// Uninstall a package by name diff --git a/src/security/secrets.rs b/src/security/secrets.rs index db9485e..ceceb4d 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -163,7 +163,7 @@ mod tests { let content = "export AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE"; let findings = scanner.scan_content(content); assert!(!findings.is_empty()); - matches!(findings[0].secret_type, SecretType::AwsAccessKey); + assert!(matches!(findings[0].secret_type, SecretType::AwsAccessKey)); } #[test] @@ -172,6 +172,10 @@ mod tests { let content = "GITHUB_TOKEN=ghp_123456789012345678901234567890123456"; let findings = scanner.scan_content(content); assert!(!findings.is_empty()); + assert!(matches!( + findings[0].secret_type, + SecretType::GitHubToken + )); } #[test] @@ -180,7 +184,7 @@ mod tests { let content = "-----BEGIN RSA PRIVATE KEY-----\nMIIEpAIBAAKCAQEA..."; let findings = scanner.scan_content(content); assert!(!findings.is_empty()); - matches!(findings[0].secret_type, SecretType::PrivateKey); + assert!(matches!(findings[0].secret_type, SecretType::PrivateKey)); } #[test] diff --git a/src/sync/conflict.rs b/src/sync/conflict.rs index 86c1388..dd529de 100644 --- a/src/sync/conflict.rs +++ b/src/sync/conflict.rs @@ -4,7 +4,6 @@ use anyhow::Result; use chrono::{DateTime, Utc}; use owo_colors::OwoColorize; use serde::{Deserialize, Serialize}; -use sha2::{Digest, Sha256}; use std::path::Path; /// Represents a file conflict during sync @@ -155,7 +154,7 @@ impl FileConflict { // Check if the file was modified if merged_path.exists() { let new_content = std::fs::read(&merged_path)?; - let new_hash = format!("{:x}", Sha256::digest(&new_content)); + let new_hash = crate::sha256_hex(&new_content); if new_hash != self.local_hash { Output::success("File was modified - using merged version"); return Ok(ConflictResolution::Merged); @@ -241,8 +240,8 @@ pub fn detect_conflict( Err(_) => return None, // Local doesn't exist, no conflict }; - let local_hash = format!("{:x}", Sha256::digest(&local_content)); - let remote_hash = format!("{:x}", Sha256::digest(remote_content)); + let local_hash = crate::sha256_hex(&local_content); + let remote_hash = crate::sha256_hex(remote_content); // No conflict if hashes match if local_hash == remote_hash { @@ -524,7 +523,7 @@ mod tests { fn test_escape_applescript_truncates_long() { let long = "a".repeat(200); let escaped = escape_applescript(&long); - assert!(escaped.len() <= 100); + assert_eq!(escaped.len(), 100); } #[test] diff --git a/src/sync/git.rs b/src/sync/git.rs index 027010d..2ceba81 100644 --- a/src/sync/git.rs +++ b/src/sync/git.rs @@ -488,10 +488,8 @@ pub fn extract_org_from_normalized_url(normalized_url: &str) -> Option { /// Generate a short checkout ID from a path (first 8 chars of SHA256 of canonical path) pub fn checkout_id_from_path(path: &Path) -> String { - use sha2::{Digest, Sha256}; let canonical = path.canonicalize().unwrap_or_else(|_| path.to_path_buf()); - let hash = Sha256::digest(canonical.to_string_lossy().as_bytes()); - format!("{:x}", hash)[..8].to_string() + crate::sha256_hex(canonical.to_string_lossy().as_bytes())[..8].to_string() } /// Check if a file is gitignored in its repository diff --git a/src/sync/mod.rs b/src/sync/mod.rs index a4f0c80..678c1e4 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -104,7 +104,6 @@ pub fn resolve_dotfile_repo_path( return flat; } } - // Default to profiled path (for new writes) profiled } @@ -1148,4 +1147,28 @@ mod tests { "plain" ); } + + #[test] + fn test_canonical_project_file_path_rejects_traversal_in_url() { + let result = canonical_project_file_path("github.com/../../etc", "config.json"); + assert!(result.is_err()); + } + + #[test] + fn test_canonical_project_file_path_rejects_traversal_in_rel_path() { + let result = canonical_project_file_path("github.com/org/repo", "../../etc/passwd"); + assert!(result.is_err()); + } + + #[test] + fn test_canonical_project_file_path_rejects_absolute_paths() { + assert!(canonical_project_file_path("/etc", "config.json").is_err()); + assert!(canonical_project_file_path("github.com/org/repo", "/etc/passwd").is_err()); + } + + #[test] + fn test_canonical_project_file_path_valid() { + let result = canonical_project_file_path("github.com/org/repo", ".env").unwrap(); + assert!(result.ends_with(".tether/projects/github.com/org/repo/.env")); + } } diff --git a/src/sync/packages.rs b/src/sync/packages.rs index 3e288ca..b1c79db 100644 --- a/src/sync/packages.rs +++ b/src/sync/packages.rs @@ -7,7 +7,6 @@ use crate::packages::{ use crate::sync::state::PackageState; use crate::sync::{MachineState, SyncState}; use anyhow::Result; -use sha2::{Digest, Sha256}; use std::collections::{HashMap, HashSet}; use std::path::Path; @@ -428,12 +427,12 @@ fn sync_brew( }; let manifest = brew_packages.generate(); - let hash = format!("{:x}", Sha256::digest(manifest.as_bytes())); + let hash = crate::sha256_hex(manifest.as_bytes()); let manifest_path = manifests_dir.join("Brewfile"); let file_hash = std::fs::read(&manifest_path) .ok() - .map(|c| format!("{:x}", Sha256::digest(&c))); + .map(|c| crate::sha256_hex(&c)); let changed = file_hash.as_ref() != Some(&hash); if !dry_run { @@ -479,12 +478,12 @@ fn sync_simple_manager( } else { packages.join("\n") + "\n" }; - let hash = format!("{:x}", Sha256::digest(manifest.as_bytes())); + let hash = crate::sha256_hex(manifest.as_bytes()); let manifest_path = manifests_dir.join(def.manifest_file); let file_hash = std::fs::read(&manifest_path) .ok() - .map(|c| format!("{:x}", Sha256::digest(&c))); + .map(|c| crate::sha256_hex(&c)); let changed = file_hash.as_ref() != Some(&hash); if !dry_run { @@ -571,7 +570,7 @@ mod tests { let pkg_state = state.packages.get("brew").unwrap(); // last_upgrade should be updated to now (newer than original) - assert!(pkg_state.last_upgrade.unwrap() > original_time); + assert!(pkg_state.last_upgrade.unwrap() >= original_time); // Other fields preserved via and_modify assert_eq!(pkg_state.last_modified, original_modified); assert_eq!(pkg_state.last_sync, original_time); From c1de6a1c5e75a581bc5d9ef760f3fc568587144b Mon Sep 17 00:00:00 2001 From: Paddo <653385+paddo@users.noreply.github.com> Date: Sat, 4 Apr 2026 14:45:27 +1000 Subject: [PATCH 2/2] style: fix rustfmt formatting in secret scanner test --- src/security/secrets.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index ceceb4d..a346ae1 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -172,10 +172,7 @@ mod tests { let content = "GITHUB_TOKEN=ghp_123456789012345678901234567890123456"; let findings = scanner.scan_content(content); assert!(!findings.is_empty()); - assert!(matches!( - findings[0].secret_type, - SecretType::GitHubToken - )); + assert!(matches!(findings[0].secret_type, SecretType::GitHubToken)); } #[test]