From bb9290c113126472be44eeae1b9d657d53246ffb Mon Sep 17 00:00:00 2001 From: DragonSlayer_14 Date: Tue, 15 Sep 2026 23:05:52 +0200 Subject: [PATCH] Fix: fstab-Setup - Mehrnutzer-sicheres Merging statt Komplett-Ersetzen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setup() ersetzte den gemeinsam genutzten verwalteten Block in /etc/fstab bei jedem Lauf komplett anhand der aktuell geladenen (nutzerspezifischen) Konfiguration. Führte ein zweiter Nutzer `sudo smart-mount setup fstab` für sein eigenes Konto aus, wurden dadurch die zuvor vom ersten Nutzer installierten Zeilen stillschweigend gelöscht. write_managed_block() führt den Block jetzt zeilenweise zusammen (jede Zeile trägt einen Tag-Kommentar mit Paar-ID/Seite/Besitzer): eigene, jetzt gelöschte Paare werden entfernt, Zeilen anderer Nutzer bleiben unangetastet. Zusätzlich behoben: - render_managed_block() brach beim ersten Paar, dessen Neuberechnung fehlschlug (z. B. ein gerade offline-MAC-adressiertes Gerät), die gesamte Blockerstellung ab - inklusive aller anderen, gesunden Paare. Fehlschläge werden jetzt pro Paar/Seite geloggt und übersprungen, die bestehende Zeile bleibt in diesem Fall erhalten. - /etc/fstab wurde per direktem std::fs::write (Trunkieren) statt atomar geschrieben - ein Absturz mitten im Schreiben konnte die Datei in einem leeren/kaputten Zustand zurücklassen. Alle Schreib- zugriffe (setup, teardown) laufen jetzt über Temp-Datei + rename. - user_home_dir() erriet bei fehlgeschlagener getent/passwd-Auflösung stillschweigend "/home/" - loggt jetzt eine Warnung. util.rs bekommt dafür extract_managed_block() als Gegenstück zu strip_managed_block(). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01LjyzpGWECyKSBWz5DtkTGz --- src/fstab/mod.rs | 294 ++++++++++++++++++++++++++++++++++++++++++++--- src/util.rs | 47 ++++++++ 2 files changed, 327 insertions(+), 14 deletions(-) diff --git a/src/fstab/mod.rs b/src/fstab/mod.rs index bd4d5e4..f4ea1c2 100644 --- a/src/fstab/mod.rs +++ b/src/fstab/mod.rs @@ -11,7 +11,8 @@ //! zur Laufzeit zwischen den beiden Backing-Verzeichnissen umschaltet (siehe //! [`crate::reconcile`]). -use std::path::PathBuf; +use std::collections::HashSet; +use std::path::{Path, PathBuf}; use std::process::Command; use crate::config::{AppConfig, DrivePair, GlobalSettings, MountContext, MountKind}; @@ -38,6 +39,17 @@ pub fn setup() -> Result<()> { ))); } + let sudo_user = std::env::var("SUDO_USER") + .ok() + .filter(|s| !s.is_empty()); + + if let Some(sudo_user) = &sudo_user + && config_ctdra::get_custom_path().is_none() + { + let user_config = user_config_path(sudo_user); + config_ctdra::set_custom_path(&user_config); + } + let cfg = crate::config::pairs::load()?; let user_pairs: Vec<&DrivePair> = cfg .pairs @@ -57,7 +69,24 @@ pub fn setup() -> Result<()> { ensure_group_membership(pair)?; } - write_managed_block(&user_pairs, &cfg.settings)?; + // Wessen zuvor installierte, jetzt aber nicht mehr konfigurierte Zeilen beim + // Zusammenführen (siehe `merge_managed_block`) entfernt werden dürfen: bei einem + // `sudo`-Aufruf im Namen eines bestimmten Nutzers ausschließlich dessen eigene Zeilen, + // sonst die Menge der in der (dann root-eigenen) Konfiguration genannten `owner_user`. + // Zeilen ANDERER Nutzer bleiben immer unangetastet - andernfalls würde ein zweiter Nutzer, + // der `setup fstab` für sein eigenes Konto ausführt, die vom ersten Nutzer installierten + // Zeilen löschen, weil sich beide denselben verwalteten Block in `/etc/fstab` teilen. + let owner_scope: Vec = match &sudo_user { + Some(u) => vec![u.clone()], + None => user_pairs + .iter() + .filter_map(|p| p.owner_user.clone()) + .collect::>() + .into_iter() + .collect(), + }; + + write_managed_block(&user_pairs, &cfg.settings, &owner_scope)?; logger_ctdra::info( "fstab", @@ -103,7 +132,7 @@ pub fn teardown() -> Result { backup(&fstab_path, &existing)?; let without_block = crate::util::strip_managed_block(&existing, BEGIN_MARKER, END_MARKER); - std::fs::write(&fstab_path, without_block).map_err(|e| Error::io(&fstab_path, e))?; + write_atomic(&fstab_path, &without_block)?; Ok(FstabTeardownOutcome::Removed) } @@ -205,23 +234,29 @@ fn ensure_group_membership(pair: &DrivePair) -> Result<()> { Ok(()) } -fn write_managed_block(pairs: &[&DrivePair], settings: &GlobalSettings) -> Result<()> { +fn write_managed_block( + pairs: &[&DrivePair], + settings: &GlobalSettings, + owner_scope: &[String], +) -> Result<()> { let fstab_path = PathBuf::from(FSTAB_PATH); let existing = std::fs::read_to_string(&fstab_path).unwrap_or_default(); backup(&fstab_path, &existing)?; let without_block = crate::util::strip_managed_block(&existing, BEGIN_MARKER, END_MARKER); - let block = render_managed_block(pairs, settings)?; + let current_block = + crate::util::extract_managed_block(&existing, BEGIN_MARKER, END_MARKER).unwrap_or_default(); + let new_block = merge_managed_block(¤t_block, pairs, settings, owner_scope); let new_contents = format!( "{}\n{}\n{}\n{}\n", without_block.trim_end(), BEGIN_MARKER, - block.trim_end(), + new_block.trim_end(), END_MARKER ); - std::fs::write(&fstab_path, new_contents).map_err(|e| Error::io(&fstab_path, e)) + write_atomic(&fstab_path, &new_contents) } fn backup(fstab_path: &PathBuf, contents: &str) -> Result<()> { @@ -231,13 +266,134 @@ fn backup(fstab_path: &PathBuf, contents: &str) -> Result<()> { Ok(()) } -fn render_managed_block(pairs: &[&DrivePair], settings: &GlobalSettings) -> Result { - let mut lines = Vec::new(); - for pair in pairs { - lines.push(fstab_line(pair, Side::Local, settings)?); - lines.push(fstab_line(pair, Side::Cloud, settings)?); +/// Schreibt `contents` atomar (Temp-Datei im selben Verzeichnis + `rename`) statt per direktem +/// Trunkieren-und-Schreiben - ein Absturz oder ein volles Dateisystem mitten im Schreiben +/// könnte `/etc/fstab` sonst in einem leeren/halb geschriebenen Zustand zurücklassen, was den +/// nächsten Boot verhindern kann. +fn write_atomic(path: &Path, contents: &str) -> Result<()> { + let tmp_path = PathBuf::from(format!("{}.smart-mount-tmp", path.display())); + std::fs::write(&tmp_path, contents).map_err(|e| Error::io(&tmp_path, e))?; + std::fs::rename(&tmp_path, path).map_err(|e| Error::io(path, e)) +} + +fn side_str(side: Side) -> &'static str { + match side { + Side::Local => "local", + Side::Cloud => "cloud", } - Ok(lines.join("\n")) +} + +/// Tag-Kommentar, der an jede von smart-mount geschriebene fstab-Zeile angehängt wird +/// (`man 5 fstab`: ein `#` leitet einen bis zum Zeilenende reichenden Kommentar ein, auch nach +/// den 6 regulären Feldern - das stört `mount(8)` nicht). Erlaubt, beim nächsten `setup fstab` +/// zeilenweise zu erkennen, zu welchem Paar/welcher Seite/welchem Besitzer eine bestehende +/// Zeile gehört, statt den kompletten Block bei jedem Lauf zu ersetzen (siehe +/// [`merge_managed_block`]). +fn line_tag(pair: &DrivePair, side: Side) -> String { + format!( + "smart-mount pair={} side={} owner={}", + pair.id, + side_str(side), + pair.owner_user.as_deref().unwrap_or("-") + ) +} + +fn tagged_line(pair: &DrivePair, side: Side, settings: &GlobalSettings) -> Result { + let line = fstab_line(pair, side, settings)?; + Ok(format!("{line} # {}", line_tag(pair, side))) +} + +/// Liest `(pair_id, side, owner)` aus dem von [`line_tag`] angehängten Kommentar einer +/// bestehenden fstab-Zeile, falls vorhanden. +fn parse_tag(line: &str) -> Option<(String, &'static str, String)> { + let marker = "# smart-mount "; + let idx = line.find(marker)?; + let rest = &line[idx + marker.len()..]; + + let mut pair_id = None; + let mut side = None; + let mut owner = None; + for token in rest.split_whitespace() { + if let Some(v) = token.strip_prefix("pair=") { + pair_id = Some(v.to_string()); + } else if let Some(v) = token.strip_prefix("side=") { + side = match v { + "local" => Some("local"), + "cloud" => Some("cloud"), + _ => None, + }; + } else if let Some(v) = token.strip_prefix("owner=") { + owner = Some(v.to_string()); + } + } + + Some((pair_id?, side?, owner.unwrap_or_else(|| "-".to_string()))) +} + +/// Führt den bestehenden verwalteten Block mit den frisch berechneten Zeilen für `pairs` +/// zusammen, statt ihn komplett zu ersetzen: +/// - eine bestehende Zeile, die zu einem der aktuell verarbeiteten Paare gehört, wird durch die +/// frische Version ersetzt (oder, falls deren Neuberechnung fehlschlägt, z. B. weil ein +/// MAC-adressiertes lokales Gerät gerade offline ist, unverändert beibehalten statt +/// ersatzlos gelöscht - siehe [`fstab_line`]/[`tagged_line`]); +/// - eine Zeile eines inzwischen aus der Konfiguration entfernten Paares DESSELBEN Nutzers +/// (`owner_scope`) wird entfernt; +/// - jede andere Zeile (insbesondere die eines ANDEREN Nutzers) bleibt unangetastet. +/// +/// Ohne diese Unterscheidung würde ein zweiter Nutzer, der `setup fstab` für sein eigenes Konto +/// ausführt, versehentlich die vom ersten Nutzer installierten Zeilen löschen, da beide +/// denselben verwalteten Block in `/etc/fstab` teilen. Ebenso würde ein einzelnes Paar, dessen +/// Neuberechnung gerade fehlschlägt, sonst den gesamten Block-Rebuild für alle anderen, +/// gesunden Paare verhindern. +fn merge_managed_block( + current_block: &str, + pairs: &[&DrivePair], + settings: &GlobalSettings, + owner_scope: &[String], +) -> String { + let current_pair_ids: HashSet<&str> = pairs.iter().map(|p| p.id.as_str()).collect(); + + let mut new_lines = Vec::new(); + let mut replaced: HashSet<(String, &'static str)> = HashSet::new(); + for pair in pairs { + for side in [Side::Local, Side::Cloud] { + match tagged_line(pair, side, settings) { + Ok(line) => { + replaced.insert((pair.id.clone(), side_str(side))); + new_lines.push(line); + } + Err(e) => { + logger_ctdra::warn( + "fstab", + &format!( + "could not compute fstab entry for pair '{}' ({}): {e} - leaving \ + any existing entry for it untouched", + pair.id, + side_str(side) + ), + ); + } + } + } + } + + let mut kept: Vec = current_block + .lines() + .filter(|line| match parse_tag(line) { + Some((pair_id, side, _)) if replaced.contains(&(pair_id.clone(), side)) => false, + Some((pair_id, _, owner)) + if !current_pair_ids.contains(pair_id.as_str()) + && owner_scope.iter().any(|o| o == &owner) => + { + false + } + _ => true, + }) + .map(str::to_string) + .collect(); + + kept.extend(new_lines); + kept.join("\n") } fn fstab_line(pair: &DrivePair, side: Side, settings: &GlobalSettings) -> Result { @@ -288,6 +444,53 @@ fn fstab_line(pair: &DrivePair, side: Side, settings: &GlobalSettings) -> Result )) } +fn user_config_path(username: &str) -> PathBuf { + let program_name = config_ctdra::get_program_name(); + let config_name = config_ctdra::get_config_name(); + let file_name = if config_name.ends_with(".toml") { + config_name + } else { + format!("{config_name}.toml") + }; + user_home_dir(username) + .map(|h| h.join(".config").join(&program_name).join(&file_name)) + .unwrap_or_else(|| { + PathBuf::from(format!("/home/{username}/.config/{program_name}/{file_name}")) + }) +} + +fn user_home_dir(username: &str) -> Option { + if let Ok(output) = Command::new("getent").args(["passwd", username]).output() + && output.status.success() + { + let stdout = String::from_utf8_lossy(&output.stdout); + let fields: Vec<&str> = stdout.trim().split(':').collect(); + if fields.len() >= 6 && !fields[5].is_empty() { + return Some(PathBuf::from(fields[5])); + } + } + if let Ok(passwd) = std::fs::read_to_string("/etc/passwd") { + for line in passwd.lines() { + let fields: Vec<&str> = line.split(':').collect(); + if fields.len() >= 6 && fields[0] == username && !fields[5].is_empty() { + return Some(PathBuf::from(fields[5])); + } + } + } + // Weder `getent` noch `/etc/passwd` konnten den Nutzer auflösen - das reine Erraten von + // `/home/` kann bei einem abweichenden Home-Verzeichnis (oder falsch geschriebenem + // Nutzernamen) dazu führen, dass `setup()` anschließend die Konfigurationsdatei am + // falschen Pfad lädt und stillschweigend "nichts zu tun" meldet. Warnen statt schweigen. + logger_ctdra::warn( + "fstab", + &format!( + "could not resolve home directory for user '{username}' via getent/passwd - \ + guessing '/home/{username}'" + ), + ); + Some(PathBuf::from(format!("/home/{username}"))) +} + /// Zeigt an, dass diese Konfiguration bereits ein einmaliges `setup fstab` benötigt hat. pub fn requires_setup(cfg: &AppConfig) -> bool { cfg.pairs.iter().any(|p| p.context == MountContext::User) @@ -345,7 +548,7 @@ mod tests { #[test] fn renders_two_lines_per_pair_each_with_its_own_unique_target() { let pair = sample_pair(); - let block = render_managed_block(&[&pair], &GlobalSettings::default()).expect("render"); + let block = merge_managed_block("", &[&pair], &GlobalSettings::default(), &[]); let lines: Vec<&str> = block.lines().collect(); assert_eq!(lines.len(), 2); @@ -377,6 +580,60 @@ mod tests { ); } + #[test] + fn merge_managed_block_preserves_lines_belonging_to_other_users() { + let pair = sample_pair(); + let other_users_line = "//other/share /backing/other cifs user,exec,noauto 0 0 \ + # smart-mount pair=other-pair side=local owner=someone-else"; + let block = merge_managed_block( + other_users_line, + &[&pair], + &GlobalSettings::default(), + &[pair.owner_user.clone().unwrap()], + ); + + assert!( + block.contains(other_users_line), + "a run scoped to one user must not touch another user's fstab lines" + ); + assert!(block.contains("pair=pair-1")); + } + + #[test] + fn merge_managed_block_drops_stale_lines_for_a_removed_pair_of_the_same_owner() { + let pair = sample_pair(); + let owner = pair.owner_user.clone().unwrap(); + let stale_line = format!( + "//old/share /backing/old cifs user,exec,noauto 0 0 \ + # smart-mount pair=deleted-pair side=local owner={owner}" + ); + // `deleted-pair` is no longer part of `pairs`, so its line should be dropped since it + // belongs to the same owner this run is scoped to - but only then. + let block = merge_managed_block(&stale_line, &[&pair], &GlobalSettings::default(), &[ + owner, + ]); + + assert!(!block.contains("deleted-pair")); + assert!(block.contains("pair=pair-1")); + } + + #[test] + fn merge_managed_block_keeps_stale_lines_of_a_different_owner() { + let pair = sample_pair(); + let stale_line = "//old/share /backing/old cifs user,exec,noauto 0 0 \ + # smart-mount pair=deleted-pair side=local owner=someone-else"; + // `owner_scope` only covers `pair.owner_user`, not `someone-else` - the stale line must + // survive even though its pair is absent from `pairs`. + let block = merge_managed_block( + stale_line, + &[&pair], + &GlobalSettings::default(), + &[pair.owner_user.clone().unwrap()], + ); + + assert!(block.contains("deleted-pair")); + } + #[test] fn strip_managed_block_removes_only_the_marked_section() { let contents = "/dev/sda1 / ext4 defaults 0 1\n# BEGIN smart-mount managed block\nfoo\n# END smart-mount managed block\n"; @@ -422,4 +679,13 @@ mod tests { create_dir_all_owned(&target, None).expect("create_dir_all_owned"); assert!(target.is_dir()); } + + #[test] + fn user_config_path_resolves_for_user() { + let user = std::env::var("USER").expect("USER env var set in test environment"); + let path = user_config_path(&user); + let s = path.to_str().unwrap(); + assert!(s.contains(&format!("/home/{user}/.config/"))); + assert!(s.ends_with("/config.toml")); + } } diff --git a/src/util.rs b/src/util.rs index cf93ea9..7870fc9 100644 --- a/src/util.rs +++ b/src/util.rs @@ -25,6 +25,35 @@ pub(crate) fn strip_managed_block(contents: &str, begin_marker: &str, end_marker out } +/// Gegenstück zu [`strip_managed_block`]: gibt nur den Inhalt *innerhalb* des Blocks zurück +/// (ohne die Marker-Zeilen selbst), oder `None`, falls kein solcher Block vorhanden ist. Für +/// ein zeilenweises Zusammenführen (statt komplettem Ersetzen) des verwalteten Blocks. +pub(crate) fn extract_managed_block( + contents: &str, + begin_marker: &str, + end_marker: &str, +) -> Option { + let mut out = String::new(); + let mut inside = false; + let mut found = false; + for line in contents.lines() { + if line.trim() == begin_marker { + inside = true; + found = true; + continue; + } + if line.trim() == end_marker { + inside = false; + continue; + } + if inside { + out.push_str(line); + out.push('\n'); + } + } + found.then_some(out) +} + #[cfg(test)] mod tests { use super::*; @@ -53,4 +82,22 @@ mod tests { "keep-me\n" ); } + + #[test] + fn extract_managed_block_returns_only_the_interior() { + let contents = "line1\n# BEGIN test\nfoo\nbar\n# END test\nline2\n"; + assert_eq!( + extract_managed_block(contents, "# BEGIN test", "# END test"), + Some("foo\nbar\n".to_string()) + ); + } + + #[test] + fn extract_managed_block_is_none_when_markers_are_absent() { + let contents = "line1\nline2\n"; + assert_eq!( + extract_managed_block(contents, "# BEGIN test", "# END test"), + None + ); + } }