diff --git a/docs/ACCEPTANCE_CHECKLIST.md b/docs/ACCEPTANCE_CHECKLIST.md index 71c3ac9..b4ca0c5 100644 --- a/docs/ACCEPTANCE_CHECKLIST.md +++ b/docs/ACCEPTANCE_CHECKLIST.md @@ -100,6 +100,12 @@ - [ ] `ensure-swap` создаёт файл по пути `swap.path` (в примере `/swapfile`) размером `swap.size_mb` (в примере 2048 MB). - [ ] Права на swap-файл — `0600`. - [ ] Перед перезаписью `/etc/fstab` создана резервная копия `/etc/fstab.awg2-harness.bak`. +- [ ] После первой правки существует `/etc/fstab.awg2-harness.original` и он **не** меняется при последующих запусках. +- [ ] Нечитаемый `/etc/fstab` (например, `chmod 000`) приводит к явной ошибке, а не к перезаписи файла одной managed-строкой. +- [ ] Отсутствующий `/etc/fstab` приводит к ошибке, а не к созданию нового файла. +- [ ] Если резервную копию записать не удалось, `/etc/fstab` остаётся неизменным. +- [ ] `status` при нечитаемом `/etc/fstab` показывает ошибку, а не «нет swap-записей». +- [ ] Путь swap с пробелом в имени попадает в `/etc/fstab` экранированным (`\040`) и корректно монтируется. - [ ] Если до запуска был активен другой swap, после `ensure-swap` он выключен. - [ ] После `ensure-swap` активен ровно один swap: `/swapfile`. - [ ] Все старые swap entries удалены из `/etc/fstab`. diff --git a/docs/IMPLEMENTATION_SPEC.md b/docs/IMPLEMENTATION_SPEC.md index 30010ac..5146b76 100644 --- a/docs/IMPLEMENTATION_SPEC.md +++ b/docs/IMPLEMENTATION_SPEC.md @@ -535,10 +535,27 @@ Production-путь перезагрузки — внешний бинарь/с - одна запись в `/etc/fstab` с настроенным приоритетом; - `vm.swappiness` из конфига применён в runtime и сохранён persistently. -При `swap.enabled = true` AWG2 Harness владеет состоянием swap на хосте эксклюзивно. Целевое состояние — ровно одна активная swap-запись: `swap.path`. Все прочие активные swap-устройства и файлы выключаются через `swapoff`. Все не-комментарные swap-записи в `/etc/fstab` удаляются и заменяются одной управляемой записью AWG2; перед перезаписью создаётся резервная копия `/etc/fstab.awg2-harness.bak`. Обычные swap-файлы, не равные настроенному `swap.path`, могут быть удалены только после успешного `swapoff`. Блочные устройства не удаляются никогда — только выключаются и убираются из fstab. Операция обязана быть идемпотентной. +При `swap.enabled = true` AWG2 Harness владеет состоянием swap на хосте эксклюзивно. Целевое состояние — ровно одна активная swap-запись: `swap.path`. Все прочие активные swap-устройства и файлы выключаются через `swapoff`. Все не-комментарные swap-записи в `/etc/fstab` удаляются и заменяются одной управляемой записью AWG2. Блочные устройства не удаляются никогда — только выключаются и убираются из fstab. Операция обязана быть идемпотентной. Создание файла: `fallocate`, при неудаче — fallback на `dd`. +### Перезапись `/etc/fstab` + +Это самая разрушительная операция пакета, поэтому она транзакционная и вынесена в отдельный модуль `fstab.rs`. Контракт: + +- ошибка чтения `/etc/fstab` **никогда** не интерпретируется как пустой файл. `PermissionDenied`, ошибка ввода-вывода и отсутствие файла различаются, и все три прекращают операцию. Отсутствие `/etc/fstab` на Debian-хосте считается аномалией, а не основанием создать его с нуля; +- пустой или состоящий из пробелов кандидат отклоняется; +- резервная копия обязательна. Если её не удалось записать, целевой файл не трогается вовсе — ситуация, в которой копия не пишется, и есть та, в которой она нужна; +- копий две: `/etc/fstab.awg2-harness.bak` перезаписывается перед каждой правкой и содержит состояние до неё, а `/etc/fstab.awg2-harness.original` создаётся один раз через `create_new` и хранит содержимое до первого вмешательства пакета; +- запись идёт во временный файл рядом с целевым, с `create_new`, `sync_all` и переносом прав с оригинала; устаревший временный файл от убитого процесса удаляется явно, а не переиспользуется; +- кандидат проверяется через `findmnt --verify --tab-file` до подмены. Если `findmnt` недоступен, шаг пропускается; если он отработал и отверг файл — операция прекращается; +- подмена — атомарный `rename`, после него каталог синхронизируется; +- после `rename` содержимое перечитывается и сверяется с записанным. Расхождение приводит к восстановлению из резервной копии. + +Путь в записи fstab экранируется: поля разделяются пробелами, поэтому пробел или табуляция в пути сдвинули бы все последующие колонки. + +Чтение swap-записей для `status` использует ту же строгую функцию чтения. Маскировка ошибки здесь показала бы «в fstab нет swap-записей», то есть здоровое состояние без drift, на хосте, где `/etc/fstab` вообще не читается. + ## 18. Тестирование Модульные тесты: diff --git a/traffic_core/src/fstab.rs b/traffic_core/src/fstab.rs new file mode 100644 index 0000000..e3f00fa --- /dev/null +++ b/traffic_core/src/fstab.rs @@ -0,0 +1,416 @@ +//! Transactional rewrite of `/etc/fstab`. +//! +//! This is the most destructive operation in the package: a bad write produces a host +//! that does not boot. Every step is therefore explicit about failure. +//! +//! Two rules drive the design: +//! +//! 1. A read that fails is never treated as an empty file. Collapsing `PermissionDenied`, +//! an I/O error and "file absent" into an empty string would make the rewrite emit a +//! fstab containing only the managed swap entry, dropping the root and boot mounts. +//! 2. The backup is mandatory. If it cannot be written, nothing is changed at all - the +//! situation where the backup fails is exactly the situation where it is needed. + +use std::{ + fs, + io::{ErrorKind, Write}, + path::{Path, PathBuf}, + process::Command, +}; + +pub const FSTAB_PATH: &str = "/etc/fstab"; + +/// Rolling backup: the state immediately before the most recent rewrite. +const BACKUP_SUFFIX: &str = ".awg2-harness.bak"; +/// Written once, with `create_new`, and never overwritten afterwards: the fstab as it was +/// before this package touched the host for the first time. +const ORIGINAL_SUFFIX: &str = ".awg2-harness.original"; +const TEMP_SUFFIX: &str = ".awg2-harness.tmp"; + +/// Reads a file that must exist. +/// +/// An absent `/etc/fstab` on a Debian host is an anomaly, not a blank slate, so it is an +/// error rather than a reason to synthesise one. +pub fn read_required(path: &Path) -> anyhow::Result { + match fs::read_to_string(path) { + Ok(content) => Ok(content), + Err(e) if e.kind() == ErrorKind::NotFound => Err(anyhow::anyhow!( + "{} does not exist; refusing to create or rewrite it", + path.display() + )), + Err(e) => Err(anyhow::anyhow!("failed to read {}: {e}", path.display())), + } +} + +/// Escapes the characters `fstab` treats as field separators. +/// +/// Fields are whitespace-separated, so a path containing a space or tab would silently +/// shift every following column. `mount` decodes these octal escapes back. +pub fn escape_field(raw: &str) -> String { + let mut out = String::with_capacity(raw.len()); + for ch in raw.chars() { + match ch { + ' ' => out.push_str("\\040"), + '\t' => out.push_str("\\011"), + '\n' => out.push_str("\\012"), + '\\' => out.push_str("\\134"), + _ => out.push(ch), + } + } + out +} + +/// An in-progress rewrite. Nothing is modified until [`FstabTransaction::commit`]. +pub struct FstabTransaction { + target: PathBuf, + original: String, +} + +impl FstabTransaction { + pub fn begin(target: &Path) -> anyhow::Result { + let original = read_required(target)?; + Ok(Self { + target: target.to_path_buf(), + original, + }) + } + + pub fn original(&self) -> &str { + &self.original + } + + /// Applies `candidate` atomically, or leaves the target byte-for-byte unchanged. + /// + /// Order matters: the backup is written and flushed before the temp file is created, + /// so there is no window in which the target is about to be replaced and no recovery + /// copy exists. + pub fn commit(self, candidate: &str) -> anyhow::Result { + if candidate == self.original { + return Ok(false); + } + if candidate.trim().is_empty() { + anyhow::bail!( + "refusing to write an empty {}: candidate content is blank", + self.target.display() + ); + } + + self.write_backups()?; + + let temp = sibling_path(&self.target, TEMP_SUFFIX)?; + if let Err(e) = self.write_candidate(&temp, candidate) { + let _ = fs::remove_file(&temp); + return Err(e); + } + + if let Err(e) = verify_candidate(&temp) { + let _ = fs::remove_file(&temp); + return Err(e); + } + + fs::rename(&temp, &self.target).map_err(|e| { + let _ = fs::remove_file(&temp); + anyhow::anyhow!( + "failed to replace {} with {}: {e}", + self.target.display(), + temp.display() + ) + })?; + + sync_parent_dir(&self.target); + + // Past the commit point: confirm what actually landed, and put the previous + // content back if it is not what we wrote. + match fs::read_to_string(&self.target) { + Ok(written) if written == candidate => Ok(true), + Ok(_) => { + self.restore_backup()?; + Err(anyhow::anyhow!( + "{} does not match the written content after rename; restored backup", + self.target.display() + )) + } + Err(e) => { + self.restore_backup()?; + Err(anyhow::anyhow!( + "failed to re-read {} after rename: {e}; restored backup", + self.target.display() + )) + } + } + } + + fn write_backups(&self) -> anyhow::Result<()> { + let backup = sibling_path(&self.target, BACKUP_SUFFIX)?; + write_file_synced(&backup, &self.original) + .map_err(|e| anyhow::anyhow!("failed to write backup {}: {e}", backup.display()))?; + + let original = sibling_path(&self.target, ORIGINAL_SUFFIX)?; + match fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(&original) + { + Ok(mut f) => { + f.write_all(self.original.as_bytes()).map_err(|e| { + anyhow::anyhow!("failed to write pristine copy {}: {e}", original.display()) + })?; + f.sync_all() + .map_err(|e| anyhow::anyhow!("failed to flush {}: {e}", original.display()))?; + } + // Already captured by an earlier run: that copy is the one to keep. + Err(e) if e.kind() == ErrorKind::AlreadyExists => {} + Err(e) => { + return Err(anyhow::anyhow!( + "failed to create pristine copy {}: {e}", + original.display() + )); + } + } + Ok(()) + } + + fn write_candidate(&self, temp: &Path, candidate: &str) -> anyhow::Result<()> { + // Stale temp from a killed run must not be silently reused. + match fs::remove_file(temp) { + Ok(()) => {} + Err(e) if e.kind() == ErrorKind::NotFound => {} + Err(e) => { + return Err(anyhow::anyhow!( + "failed to clear stale {}: {e}", + temp.display() + )); + } + } + + let mut f = fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(temp) + .map_err(|e| anyhow::anyhow!("failed to create {}: {e}", temp.display()))?; + f.write_all(candidate.as_bytes()) + .map_err(|e| anyhow::anyhow!("failed to write {}: {e}", temp.display()))?; + f.sync_all() + .map_err(|e| anyhow::anyhow!("failed to flush {}: {e}", temp.display()))?; + drop(f); + + copy_permissions(&self.target, temp)?; + Ok(()) + } + + fn restore_backup(&self) -> anyhow::Result<()> { + let backup = sibling_path(&self.target, BACKUP_SUFFIX)?; + fs::copy(&backup, &self.target).map_err(|e| { + anyhow::anyhow!( + "rollback failed: could not restore {} from {}: {e}", + self.target.display(), + backup.display() + ) + })?; + sync_parent_dir(&self.target); + Ok(()) + } +} + +fn sibling_path(target: &Path, suffix: &str) -> anyhow::Result { + let name = target + .file_name() + .ok_or_else(|| anyhow::anyhow!("invalid fstab path: {}", target.display()))?; + let mut sibling = name.to_os_string(); + sibling.push(suffix); + Ok(target.with_file_name(sibling)) +} + +fn write_file_synced(path: &Path, content: &str) -> std::io::Result<()> { + let mut f = fs::File::create(path)?; + f.write_all(content.as_bytes())?; + f.sync_all() +} + +/// Asks `findmnt` whether the candidate parses as a valid tab file. +/// +/// Skipped when `findmnt` cannot be spawned; when it does run, a rejection is fatal. +fn verify_candidate(temp: &Path) -> anyhow::Result<()> { + let out = Command::new("findmnt") + .args(["--verify", "--tab-file"]) + .arg(temp) + .output(); + match out { + Ok(o) if o.status.success() => Ok(()), + Ok(o) => Err(anyhow::anyhow!( + "candidate fstab rejected by findmnt --verify: {}{}", + String::from_utf8_lossy(&o.stdout).trim(), + String::from_utf8_lossy(&o.stderr).trim() + )), + Err(_) => Ok(()), + } +} + +#[cfg(unix)] +fn copy_permissions(from: &Path, to: &Path) -> anyhow::Result<()> { + let md = fs::metadata(from) + .map_err(|e| anyhow::anyhow!("failed to stat {}: {e}", from.display()))?; + fs::set_permissions(to, md.permissions()) + .map_err(|e| anyhow::anyhow!("failed to set permissions on {}: {e}", to.display())) +} + +#[cfg(not(unix))] +fn copy_permissions(_from: &Path, _to: &Path) -> anyhow::Result<()> { + Ok(()) +} + +/// Best-effort: the rename already made the change visible, and there is no useful +/// recovery if flushing the directory entry fails. +#[cfg(unix)] +fn sync_parent_dir(target: &Path) { + if let Some(dir) = target.parent() + && let Ok(f) = fs::File::open(dir) + { + let _ = f.sync_all(); + } +} + +#[cfg(not(unix))] +fn sync_parent_dir(_target: &Path) {} + +#[cfg(test)] +mod tests { + use super::*; + + fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("awg2_fstab_{}_{}", name, std::process::id())); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).expect("scratch dir"); + dir + } + + fn seed(dir: &Path, content: &str) -> PathBuf { + let p = dir.join("fstab"); + fs::write(&p, content).expect("seed"); + p + } + + const SAMPLE: &str = "UUID=aaaa / ext4 defaults 0 1\nUUID=bbbb /boot ext4 defaults 0 2\n"; + + /// The regression that motivates this module: an unreadable fstab must never be + /// treated as empty, because the rewrite would then drop every real mount. + #[test] + fn unreadable_target_is_an_error_not_an_empty_file() { + let dir = scratch("unreadable"); + // A directory is readable as a path but never as a string, on every platform. + let err = read_required(&dir).expect_err("must fail"); + assert!(!err.to_string().contains("does not exist")); + } + + #[test] + fn missing_target_is_an_error() { + let dir = scratch("missing"); + let err = read_required(&dir.join("nope")).expect_err("must fail"); + assert!(err.to_string().contains("does not exist")); + } + + #[test] + fn begin_fails_when_target_is_missing() { + let dir = scratch("begin_missing"); + assert!(FstabTransaction::begin(&dir.join("nope")).is_err()); + } + + #[test] + fn commit_writes_candidate_and_backup() { + let dir = scratch("commit"); + let target = seed(&dir, SAMPLE); + let candidate = format!("{SAMPLE}/swapfile none swap sw,pri=1 0 0\n"); + + let tx = FstabTransaction::begin(&target).expect("begin"); + assert!(tx.commit(&candidate).expect("commit")); + + assert_eq!(fs::read_to_string(&target).expect("read"), candidate); + let backup = dir.join(format!("fstab{BACKUP_SUFFIX}")); + assert_eq!(fs::read_to_string(backup).expect("backup"), SAMPLE); + let original = dir.join(format!("fstab{ORIGINAL_SUFFIX}")); + assert_eq!(fs::read_to_string(original).expect("original"), SAMPLE); + } + + #[test] + fn unchanged_candidate_is_a_no_op() { + let dir = scratch("noop"); + let target = seed(&dir, SAMPLE); + let tx = FstabTransaction::begin(&target).expect("begin"); + assert!(!tx.commit(SAMPLE).expect("commit")); + assert!(!dir.join(format!("fstab{BACKUP_SUFFIX}")).exists()); + } + + #[test] + fn empty_candidate_is_refused() { + let dir = scratch("empty"); + let target = seed(&dir, SAMPLE); + let tx = FstabTransaction::begin(&target).expect("begin"); + let err = tx.commit(" \n").expect_err("must refuse"); + assert!(err.to_string().contains("refusing to write an empty")); + assert_eq!(fs::read_to_string(&target).expect("read"), SAMPLE); + } + + /// The pristine copy records the host as it was before the package first ran, so a + /// later rewrite must not overwrite it with an already-managed fstab. + #[test] + fn pristine_copy_is_written_once() { + let dir = scratch("pristine"); + let target = seed(&dir, SAMPLE); + let first = format!("{SAMPLE}/swapfile none swap sw,pri=1 0 0\n"); + FstabTransaction::begin(&target) + .expect("begin") + .commit(&first) + .expect("commit"); + + let second = format!("{SAMPLE}/swapfile none swap sw,pri=5 0 0\n"); + FstabTransaction::begin(&target) + .expect("begin") + .commit(&second) + .expect("commit"); + + let original = dir.join(format!("fstab{ORIGINAL_SUFFIX}")); + assert_eq!(fs::read_to_string(original).expect("original"), SAMPLE); + let backup = dir.join(format!("fstab{BACKUP_SUFFIX}")); + assert_eq!(fs::read_to_string(backup).expect("backup"), first); + } + + #[test] + fn stale_temp_file_does_not_block_commit() { + let dir = scratch("stale"); + let target = seed(&dir, SAMPLE); + fs::write(dir.join(format!("fstab{TEMP_SUFFIX}")), "garbage").expect("stale temp"); + + let candidate = format!("{SAMPLE}/swapfile none swap sw,pri=1 0 0\n"); + FstabTransaction::begin(&target) + .expect("begin") + .commit(&candidate) + .expect("commit"); + assert_eq!(fs::read_to_string(&target).expect("read"), candidate); + assert!(!dir.join(format!("fstab{TEMP_SUFFIX}")).exists()); + } + + /// Backup lives in the target directory: an unwritable directory must stop the + /// transaction before the target is touched. + #[test] + fn backup_failure_leaves_target_untouched() { + let dir = scratch("backup_fail"); + let target = seed(&dir, SAMPLE); + // Occupy the backup name with a directory so writing the backup cannot succeed. + fs::create_dir(dir.join(format!("fstab{BACKUP_SUFFIX}"))).expect("blocker"); + + let tx = FstabTransaction::begin(&target).expect("begin"); + let err = tx + .commit(&format!("{SAMPLE}/swapfile none swap sw,pri=1 0 0\n")) + .expect_err("must fail"); + assert!(err.to_string().contains("failed to write backup")); + assert_eq!(fs::read_to_string(&target).expect("read"), SAMPLE); + } + + #[test] + fn escapes_fstab_field_separators() { + assert_eq!(escape_field("/var/my swap"), "/var/my\\040swap"); + assert_eq!(escape_field("/a\tb"), "/a\\011b"); + assert_eq!(escape_field("/a\\b"), "/a\\134b"); + assert_eq!(escape_field("/swapfile"), "/swapfile"); + } +} diff --git a/traffic_core/src/main.rs b/traffic_core/src/main.rs index 595a625..c09425d 100644 --- a/traffic_core/src/main.rs +++ b/traffic_core/src/main.rs @@ -4,6 +4,7 @@ mod daemon; mod docker; mod error; mod firewall; +mod fstab; mod iface; mod lock; mod netns; diff --git a/traffic_core/src/swap.rs b/traffic_core/src/swap.rs index c8edfa7..06a6552 100644 --- a/traffic_core/src/swap.rs +++ b/traffic_core/src/swap.rs @@ -1,6 +1,5 @@ use std::{ fs, - io::Write, path::{Path, PathBuf}, process::Command, time::Duration, @@ -11,7 +10,11 @@ use std::os::unix::fs::PermissionsExt; use anyhow::Context; -use crate::{config::Config, lock::FileLock}; +use crate::{ + config::Config, + fstab::{self, FSTAB_PATH, FstabTransaction, escape_field}, + lock::FileLock, +}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct ActiveSwap { @@ -287,22 +290,9 @@ fn create_swapfile(path: &Path, size_bytes: u64) -> anyhow::Result<()> { } fn rewrite_fstab_exclusive(desired: &DesiredSwap) -> anyhow::Result<()> { - let fstab_path = "/etc/fstab"; - let bak_path = "/etc/fstab.awg2-harness.bak"; - let tmp_path = "/etc/fstab.awg2-harness.tmp"; - - let content = fs::read_to_string(fstab_path).unwrap_or_default(); - let rewritten = rewrite_fstab_content(&content, &desired.path, desired.priority); - if rewritten == content { - return Ok(()); - } - - let _ = fs::write(bak_path, &content); - let mut f = fs::File::create(tmp_path)?; - f.write_all(rewritten.as_bytes())?; - f.sync_all()?; - drop(f); - fs::rename(tmp_path, fstab_path)?; + let tx = FstabTransaction::begin(Path::new(FSTAB_PATH))?; + let rewritten = rewrite_fstab_content(tx.original(), &desired.path, desired.priority); + tx.commit(&rewritten)?; Ok(()) } @@ -332,8 +322,14 @@ fn rewrite_fstab_content(content: &str, desired_path: &Path, priority: i32) -> S s } +/// Path goes through fstab escaping: fields are whitespace-separated, so an unescaped +/// space in the path would shift every following column. fn render_fstab_line(path: &Path, priority: i32) -> String { - format!("{} none swap sw,pri={} 0 0\n", path.display(), priority) + format!( + "{} none swap sw,pri={} 0 0\n", + escape_field(&path.to_string_lossy()), + priority + ) } fn enable_desired_swap(desired: &DesiredSwap) -> anyhow::Result<()> { @@ -472,8 +468,13 @@ fn verify_exclusive_swap_state(desired: &DesiredSwap) -> anyhow::Result<()> { Ok(()) } +/// Reads swap entries for inspection. +/// +/// Strict for the same reason the rewrite is: masking a read failure here would report +/// "no swap entries in fstab", which reads as healthy drift-free state on a host whose +/// fstab could not be read at all. fn read_fstab_swap_entries(path: &str) -> anyhow::Result> { - let content = fs::read_to_string(path).unwrap_or_default(); + let content = fstab::read_required(Path::new(path))?; let mut out = Vec::new(); for line in content.lines() { let t = line.trim();