fix(swap): make the /etc/fstab rewrite transactional [P0]

Two release blockers, both in the most destructive operation the package performs.

1. An unreadable /etc/fstab was treated as an empty one.

   let content = fs::read_to_string(fstab_path).unwrap_or_default();

   PermissionDenied, an I/O error, a damaged filesystem and "file absent" all collapsed
   into an empty string. The rewrite then emitted a fstab containing only the managed swap
   line and installed it atomically and successfully, dropping the root and boot mounts.
   Consequence: a host that does not boot, guarded by one unwrap_or_default. The same
   masking was present in read_fstab_swap_entries, so an unreadable fstab was reported by
   status as "no swap entries", which reads as healthy drift-free state.

2. The backup was best-effort.

   let _ = fs::write(bak_path, &content);

   If it failed, the rewrite proceeded anyway. IMPLEMENTATION_SPEC and the acceptance
   checklist both asserted the backup exists. The scenario where the backup fails is
   exactly the scenario where it is needed.

New module fstab.rs owns the operation:

- read_required distinguishes NotFound from every other error and fails on both. A
  missing /etc/fstab on Debian is an anomaly, not a blank slate, so it is never created;
- a blank candidate is refused outright;
- the backup is mandatory and flushed before the temp file is created, so there is no
  window where the target is about to be replaced and no recovery copy exists;
- two copies are kept: .awg2-harness.bak is the state before the current rewrite,
  .awg2-harness.original is written once with create_new and holds the host as it was
  before this package first touched it;
- the temp file is a sibling of the target, created with create_new, sync_all'd, and
  inherits the original permissions; a stale temp from a killed run is removed explicitly
  rather than reused;
- the candidate is validated with findmnt --verify --tab-file before the swap. The step is
  skipped when findmnt cannot be spawned; a rejection is fatal;
- commit is an atomic rename followed by a directory fsync;
- after the rename the target is re-read and compared to what was written. A mismatch
  restores the backup and fails.

Also fixed: render_fstab_line interpolated the path raw. fstab fields are
whitespace-separated, so a space or tab in swap.path would shift every following column.
Paths now go through octal escaping (\040, \011, \012, \134).

Ten tests, all portable - the "unreadable but present" case is exercised by pointing the
reader at a directory, which fails as a non-NotFound error on every platform. Covered:
unreadable target is an error rather than an empty file, missing target is an error,
backup failure leaves the target byte-identical, blank candidate refused, unchanged
candidate is a no-op that writes no backup, the pristine copy survives repeated rewrites,
a stale temp does not block commit, and field escaping.

Docs updated: IMPLEMENTATION_SPEC section 17 gained the full rewrite contract, the
acceptance checklist gained seven checks including the two failure modes above.

cargo test --workspace: 124 + 4 passed, ten new. clippy -D warnings clean.
This commit is contained in:
2026-08-06 21:02:38 +05:00
parent 7254ebe9e4
commit 549fbcae31
5 changed files with 462 additions and 21 deletions
+21 -20
View File
@@ -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<Vec<String>> {
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();