hive-priv: publish config files by rename, bound the toplevel build

Every file hive-priv writes for another reader now goes through one
StagedFile: a temp with a unique dot-prefixed `.partial` name in the
destination directory, created O_EXCL|O_NOFOLLOW at its final mode
(and owner, where one is set), fsynced, renamed onto the final name
relative to a directory fd, then the directory fsynced. Dropping it
unpublished unlinks the temp.

- write_resource_limits wrote its systemd drop-in in place, so a
  concurrent daemon-reload could load a truncated file.
- sync_agent_tmpfiles staged through a fixed `.tmp` name, so two
  overlapping syncs shared one inode and one could publish the other's
  bytes, or a mix.
- write_nspawn_flags rewrote /etc/nixos-containers/<c>.conf in place;
  it now keeps the file's existing owner and mode.
- write_agent_dir_file (agent tokens, sidecars, pause marker) already
  renamed, but through a fixed temp opened O_TRUNC, with no fsync.
- write_bridge_dns_marker_in truncated and wrote the marker in place;
  a leaf the container planted (symlink, FIFO) is now replaced by the
  rename instead of refused.
- register_ci_runner must keep writing in place (nspawn pins the
  bind-mounted inode); it now creates the file 0600 when the tmpfiles
  seed is missing, instead of at the umask's mode until the chmod.

nix_build_toplevel now runs nix as its own process-group leader and,
after one hour, SIGKILLs the group and fails with "timed out after
3600s". One hour is four times CI's observed cold-cache flake check.

Refs #4723
This commit is contained in:
atlas 2026-09-26 19:39:50 +02:00 • committed by mara
commit fc85d0ee7e

View file

@ -864,43 +864,53 @@ fn toplevel_attr(name: &str) -> String {
format!("{META_DIR}#nixosConfigurations.{name}.config.system.build.toplevel") format!("{META_DIR}#nixosConfigurations.{name}.config.system.build.toplevel")
} }
/// Build `nixosConfigurations.<name>.config.system.build.toplevel` and /// Wall-clock bound on [`nix_build_toplevel`]. Four times CI's observed
/// return the resulting store path, so `create`/`update` can hand /// cold-cache `nix flake check` (~15 min, `.forgejo/workflows/ci.yml`), which
/// `nixos-container` an explicit `--system-path` instead of letting its /// compiles this workspace's crates, the part of an agent toplevel no binary
/// own `buildFlake()` build to a racy shared out-link. See "Container /// cache holds; the rebuild path has usually warmed this exact attr already
/// toplevel builds" in this crate's README for the full story — the /// (`hive-c0re`'s `prebuild_toplevel`). A stalled download is bounded by nix's
/// concurrency bug this closes (the "agent container gets closure of /// own `stalled-download-timeout`; this covers the rest, such as a wedged
/// other agent" mystery bug) and why stdout/stderr are drained /// nix-daemon holding the caller's build slot forever.
/// concurrently but handled asymmetrically (stderr streamed live, const NIX_BUILD_TIMEOUT: std::time::Duration = std::time::Duration::from_hours(1);
/// stdout captured and required to be exactly one line).
/// How a [`run_bounded`] child ended.
enum BoundedRun {
Exited {
status: std::process::ExitStatus,
stdout: String,
stderr: String,
},
/// Still running at the limit; its whole process group was killed.
TimedOut,
}
/// Run `cmd` to completion or until `limit`, draining both pipes: stdout is
/// captured silently, stderr is logged and, when `writer` is set, forwarded
/// live as [`PrivEvent::Line`]s.
/// ///
/// `writer` is `None` for the non-streaming call shape (`stream: false`); /// The child leads its own process group and a timeout `SIGKILL`s the whole
/// stderr still logs to journald either way, just without the /// group, so a grandchild still holding the pipes (or the daemon connection)
/// `PrivEvent::Line` forwarding. /// dies with it instead of outliving the error.
async fn nix_build_toplevel(name: &str, mut writer: Option<&mut OwnedWriteHalf>) -> Result<String> { async fn run_bounded(
mut cmd: Command,
limit: std::time::Duration,
mut writer: Option<&mut OwnedWriteHalf>,
) -> std::io::Result<BoundedRun> {
use tokio::io::AsyncBufReadExt as _; use tokio::io::AsyncBufReadExt as _;
let attr = toplevel_attr(name); let mut child = cmd
let args = [
"--extra-experimental-features",
"nix-command flakes",
"build",
"--no-link",
"--print-out-paths",
&attr,
];
let mut child = Command::new("nix")
.args(args)
.stdout(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped())
.spawn() .process_group(0)
.with_context(|| format!("invoke nix build {attr}"))?; .spawn()?;
// Group leader, so its pid is the pgid.
let pgid = child.id().and_then(|pid| i32::try_from(pid).ok());
let stdout = child.stdout.take().expect("stdout piped"); let stdout = child.stdout.take().expect("stdout piped");
let stderr = child.stderr.take().expect("stderr piped"); let stderr = child.stderr.take().expect("stderr piped");
let run = async {
let mut stdout_lines = BufReader::new(stdout).lines(); let mut stdout_lines = BufReader::new(stdout).lines();
let mut stderr_lines = BufReader::new(stderr).lines(); let mut stderr_lines = BufReader::new(stderr).lines();
let mut stdout_buf = String::new(); let mut stdout_buf = String::new();
let mut stderr_buf = String::new(); let mut stderr_buf = String::new();
@ -933,8 +943,8 @@ async fn nix_build_toplevel(name: &str, mut writer: Option<&mut OwnedWriteHalf>)
} }
line = stderr_lines.next_line(), if !stderr_done => { line = stderr_lines.next_line(), if !stderr_done => {
match line { match line {
// Streamed as it arrives — the progress the dashboard // Streamed as it arrives, so the dashboard and
// and `journalctl -f` were missing. // `journalctl -f` show build progress live.
Ok(Some(l)) => { Ok(Some(l)) => {
tracing::info!(target: "nix-build-toplevel", "{l}"); tracing::info!(target: "nix-build-toplevel", "{l}");
if let Some(w) = writer.as_deref_mut() { if let Some(w) = writer.as_deref_mut() {
@ -954,13 +964,68 @@ async fn nix_build_toplevel(name: &str, mut writer: Option<&mut OwnedWriteHalf>)
} }
} }
} }
let status = child.wait().await?;
Ok::<_, std::io::Error>(BoundedRun::Exited {
status,
stdout: stdout_buf,
stderr: stderr_buf,
})
};
if let Ok(exited) = tokio::time::timeout(limit, run).await {
return exited;
}
if let Some(pgid) = pgid {
// SAFETY: plain `kill(2)`; `-pgid` targets the child's own process
// group, which `process_group(0)` made it lead.
unsafe {
libc::kill(-pgid, libc::SIGKILL);
}
}
child.wait().await?;
Ok(BoundedRun::TimedOut)
}
/// Build `nixosConfigurations.<name>.config.system.build.toplevel` and
/// return the resulting store path, so `create`/`update` can hand
/// `nixos-container` an explicit `--system-path` instead of letting its
/// own `buildFlake()` build to a racy shared out-link. See "Container
/// toplevel builds" in this crate's README for the full story — the
/// concurrency bug this closes (the "agent container gets closure of
/// other agent" mystery bug) and why stdout/stderr are drained
/// concurrently but handled asymmetrically (stderr streamed live,
/// stdout captured and required to be exactly one line).
///
/// `writer` is `None` for the non-streaming call shape (`stream: false`);
/// stderr still logs to journald either way, just without the
/// `PrivEvent::Line` forwarding. Bounded by [`NIX_BUILD_TIMEOUT`].
async fn nix_build_toplevel(name: &str, writer: Option<&mut OwnedWriteHalf>) -> Result<String> {
let attr = toplevel_attr(name);
let mut cmd = Command::new("nix");
cmd.args([
"--extra-experimental-features",
"nix-command flakes",
"build",
"--no-link",
"--print-out-paths",
&attr,
]);
let (status, stdout_buf, stderr_buf) = match run_bounded(cmd, NIX_BUILD_TIMEOUT, writer)
.await
.with_context(|| format!("run nix build {attr}"))?
{
BoundedRun::Exited {
status,
stdout,
stderr,
} => (status, stdout, stderr),
BoundedRun::TimedOut => bail!(
"nix build {attr} timed out after {}s; killed it",
NIX_BUILD_TIMEOUT.as_secs()
),
};
// ⚠️ Success is decided by the exit status, not by "we parsed a // ⚠️ Success is decided by the exit status, not by "we parsed a
// path" — a build can print to stdout and still fail. // path" — a build can print to stdout and still fail.
let status = child
.wait()
.await
.with_context(|| format!("wait nix build {attr}"))?;
if !status.success() { if !status.success() {
bail!( bail!(
"nix build {attr} failed ({status}): {}", "nix build {attr} failed ({status}): {}",
@ -1102,7 +1167,9 @@ fn write_resource_limits(
std::fs::create_dir_all(&dir).with_context(|| format!("create {dir}"))?; std::fs::create_dir_all(&dir).with_context(|| format!("create {dir}"))?;
let path = format!("{dir}/hyperhive-limits.conf"); let path = format!("{dir}/hyperhive-limits.conf");
let content = limits_dropin_body(&runtime_dir, memory_max, cpu_quota, cpu_weight, io_weight); let content = limits_dropin_body(&runtime_dir, memory_max, cpu_quota, cpu_weight, io_weight);
std::fs::write(&path, content).with_context(|| format!("write {path}"))?; // Published whole: a `daemon-reload` between truncate and write would
// otherwise load the unit with its limits and start guard missing.
publish_file(Path::new(&path), content.as_bytes(), 0o644, None)?;
Ok((String::new(), String::new())) Ok((String::new(), String::new()))
} }
@ -1248,7 +1315,8 @@ fn clear_runner_credentials(path: &str) -> Result<()> {
/// the registration token c0re passes here is written, and it lands on a host /// the registration token c0re passes here is written, and it lands on a host
/// path bind-mounted read-only into hive-ci. /// path bind-mounted read-only into hive-ci.
async fn register_ci_runner(token: &str) -> Result<(String, String)> { async fn register_ci_runner(token: &str) -> Result<(String, String)> {
use std::os::unix::fs::PermissionsExt as _; use std::io::Write as _;
use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _};
// Reject anything that could corrupt the `KEY=VALUE` env-file or smuggle a // Reject anything that could corrupt the `KEY=VALUE` env-file or smuggle a
// second line — a forge registration token is an opaque single-line string. // second line — a forge registration token is an opaque single-line string.
if token.is_empty() || token.contains(['\n', '\r', '\0']) { if token.is_empty() || token.contains(['\n', '\r', '\0']) {
@ -1259,8 +1327,15 @@ async fn register_ci_runner(token: &str) -> Result<(String, String)> {
// `echo > $FILE`), NOT a temp+rename: nspawn pins this file's inode into // `echo > $FILE`), NOT a temp+rename: nspawn pins this file's inode into
// hive-ci at container start, so a rename would leave the running runner // hive-ci at container start, so a rename would leave the running runner
// reading the old content. Format + perms match the tmpfiles seed and the // reading the old content. Format + perms match the tmpfiles seed and the
// prefetch: `TOKEN=<tok>`, mode 0600, root-owned. // prefetch: `TOKEN=<tok>`, mode 0600, root-owned. Created 0600 when the
std::fs::write(token_path, format!("TOKEN={token}\n")) // seed is missing, so the token is never on disk at a wider mode.
std::fs::OpenOptions::new()
.write(true)
.create(true)
.truncate(true)
.mode(0o600)
.open(token_path)
.and_then(|mut f| f.write_all(format!("TOKEN={token}\n").as_bytes()))
.with_context(|| format!("write {token_path}"))?; .with_context(|| format!("write {token_path}"))?;
std::fs::set_permissions(token_path, std::fs::Permissions::from_mode(0o600)) std::fs::set_permissions(token_path, std::fs::Permissions::from_mode(0o600))
.with_context(|| format!("chmod {token_path}"))?; .with_context(|| format!("chmod {token_path}"))?;
@ -1364,44 +1439,166 @@ fn matrix_token_filename(account: Option<&str>) -> String {
} }
} }
/// Name the content is written under before being renamed onto `filename`. /// Name the content is written under before being renamed onto `filename`,
/// The leading dot is load-bearing rather than tidy: `nix/agent-modules/ /// unique per call so two concurrent writes of one file never share a temp.
/// matrix.nix` starts the agent's matrix daemon on the glob `matrix-token*`, ///
/// which a `<name>.partial` suffix would match — waking it on precisely the /// The leading dot and the `.partial` suffix are load-bearing:
/// empty file the rename exists to hide. /// `nix/agent-modules/matrix.nix` starts the agent's matrix daemon on the glob
/// `matrix-token*`, and systemd reads only `*.conf` out of `tmpfiles.d` and
/// drop-in dirs, so none of them can pick up a half-written temp.
fn partial_name(filename: &str) -> String { fn partial_name(filename: &str) -> String {
format!(".{filename}.partial") use std::sync::atomic::{AtomicU64, Ordering};
static SEQ: AtomicU64 = AtomicU64::new(0);
let n = SEQ.fetch_add(1, Ordering::Relaxed);
format!(".{filename}.{}-{n}.partial", std::process::id())
} }
/// Create/overwrite `dir/filename` at 0600 without following a symlink at the /// Open `path` as a directory fd for [`StagedFile`] to work relative to.
/// leaf, returning the open fd for the caller to `fchown`. `filename` must be a fn open_dir(path: &Path) -> Result<std::fs::File> {
/// single plain component (no `/`, `.`, `..`) — the leaf sits in an use std::os::unix::fs::OpenOptionsExt as _;
/// agent-writable dir, so `O_NOFOLLOW` refuses a planted symlink (`ELOOP`) std::fs::OpenOptions::new()
/// instead of letting this root-privileged write/chmod be redirected at another .read(true)
/// file; `O_WRONLY` refuses a directory leaf (`EISDIR`); `O_TRUNC` keeps the .custom_flags(libc::O_DIRECTORY)
/// overwrite semantics for an existing regular file. `.mode(0o600)` sets the .open(path)
/// create mode; the explicit `fchmod` after (on the fd, not a re-resolved path) .with_context(|| format!("open directory {}", path.display()))
/// tightens an already-existing file and dodges umask. The returned fd is the }
/// exact inode the write hit, so the caller's `fchown` is TOCTOU-immune.
fn write_state_file_nofollow(dir: &Path, filename: &str, content: &str) -> Result<std::fs::File> {
use std::io::Write as _;
use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _};
ensure_plain_filename("write_state_file_nofollow", filename)?; /// A file written under a [`partial_name`] in its destination directory and
let path = dir.join(filename); /// then renamed onto its final name, so a reader sees either the old file or
let mut file = std::fs::OpenOptions::new() /// the complete new one, never a partial or wrong-mode one. Mode, and owner
.write(true) /// when the caller `fchown`s [`file`](Self::file), are on the inode before the
.create(true) /// rename makes it visible.
.truncate(true) ///
.mode(0o600) /// Every step is relative to the `dir` fd, so it is safe in a directory an
.custom_flags(libc::O_NOFOLLOW) /// agent or container controls: the temp is created `O_EXCL|O_NOFOLLOW`, which
.open(&path) /// refuses any entry already planted at its name, and `rename` replaces
.with_context(|| format!("open (no-follow) {}", path.display()))?; /// whatever sits at the final name without following it.
file.write_all(content.as_bytes()) ///
.with_context(|| format!("write {}", path.display()))?; /// Dropped without [`publish`](Self::publish), it unlinks the temp, so a
file.set_permissions(std::fs::Permissions::from_mode(0o600)) /// failure between the two leaves the old file untouched and no residue.
.with_context(|| format!("chmod 600 {}", path.display()))?; struct StagedFile<'a> {
Ok(file) dir: &'a std::fs::File,
tmp: std::ffi::CString,
dest: std::ffi::CString,
dest_path: PathBuf,
file: std::fs::File,
published: bool,
}
impl<'a> StagedFile<'a> {
/// Create the temp at `mode` and write `content` to it. The mode is set
/// again on the fd after the write, so the umask cannot change it.
fn write(
dir: &'a std::fs::File,
dir_path: &Path,
filename: &str,
content: &[u8],
mode: u32,
) -> Result<Self> {
use std::io::Write as _;
use std::os::unix::fs::PermissionsExt as _;
ensure_plain_filename("StagedFile::write", filename)?;
let dest_path = dir_path.join(filename);
let tmp_name = partial_name(filename);
let tmp = std::ffi::CString::new(tmp_name.as_str())
.with_context(|| format!("NUL in filename {filename:?}"))?;
let dest = std::ffi::CString::new(filename)
.with_context(|| format!("NUL in filename {filename:?}"))?;
// SAFETY: `dir` is an open directory fd and `tmp` is NUL-terminated;
// the result is checked before it is wrapped.
let fd = unsafe {
libc::openat(
dir.as_raw_fd(),
tmp.as_ptr(),
libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL | libc::O_NOFOLLOW | libc::O_CLOEXEC,
mode,
)
};
if fd < 0 {
return Err(std::io::Error::last_os_error())
.with_context(|| format!("create {}", dir_path.join(&tmp_name).display()));
}
// SAFETY: `fd` is a fresh, valid fd that nothing else owns.
let file = std::fs::File::from(unsafe { OwnedFd::from_raw_fd(fd) });
let mut staged = Self {
dir,
tmp,
dest,
dest_path,
file,
published: false,
};
staged
.file
.write_all(content)
.with_context(|| format!("write {}", staged.dest_path.display()))?;
staged
.file
.set_permissions(std::fs::Permissions::from_mode(mode))
.with_context(|| format!("chmod {mode:o} {}", staged.dest_path.display()))?;
Ok(staged)
}
/// The temp's open fd, for an `fchown` that must land before publishing.
fn file(&self) -> &std::fs::File {
&self.file
}
/// Flush the content, rename the temp onto the final name, and flush the
/// directory: without the last step a crash can lose the rename itself.
fn publish(mut self) -> Result<()> {
self.file
.sync_all()
.with_context(|| format!("fsync {}", self.dest_path.display()))?;
let fd = self.dir.as_raw_fd();
// SAFETY: `fd` is an open directory fd; both names are NUL-terminated.
if unsafe { libc::renameat(fd, self.tmp.as_ptr(), fd, self.dest.as_ptr()) } != 0 {
return Err(std::io::Error::last_os_error())
.with_context(|| format!("publishing {}", self.dest_path.display()));
}
self.published = true;
self.dir
.sync_all()
.with_context(|| format!("fsync directory of {}", self.dest_path.display()))
}
}
impl Drop for StagedFile<'_> {
fn drop(&mut self) {
if self.published {
return;
}
// SAFETY: `self.dir` is an open directory fd; `self.tmp` is
// NUL-terminated.
if unsafe { libc::unlinkat(self.dir.as_raw_fd(), self.tmp.as_ptr(), 0) } != 0 {
tracing::warn!(
dest = %self.dest_path.display(),
error = %std::io::Error::last_os_error(),
"failed to remove unpublished temp"
);
}
}
}
/// Publish `content` at `path` through a [`StagedFile`], at `mode` and, when
/// given, owned by `owner` (`(uid, gid)`).
fn publish_file(path: &Path, content: &[u8], mode: u32, owner: Option<(u32, u32)>) -> Result<()> {
let (Some(dir_path), Some(filename)) =
(path.parent(), path.file_name().and_then(|n| n.to_str()))
else {
bail!(
"publish_file: {} has no directory or file name",
path.display()
);
};
let dir = open_dir(dir_path)?;
let staged = StagedFile::write(&dir, dir_path, filename, content, mode)?;
if let Some((uid, gid)) = owner {
std::os::unix::fs::fchown(staged.file(), Some(uid), Some(gid))
.with_context(|| format!("chown {}", path.display()))?;
}
staged.publish()
} }
/// Sidecar written alongside an extra matrix account's token /// Sidecar written alongside an extra matrix account's token
@ -1469,8 +1666,7 @@ fn set_agent_paused(agent_name: &str, paused: bool) -> Result<(String, String)>
/// ///
/// `remove_file` unlinks the leaf itself and never follows a symlink, so an /// `remove_file` unlinks the leaf itself and never follows a symlink, so an
/// agent-planted link at the marker path cannot redirect this root unlink /// agent-planted link at the marker path cannot redirect this root unlink
/// at another file — the same threat `write_state_file_nofollow` closes on /// at another file — the same threat [`StagedFile`] closes on the create side.
/// the create side.
fn remove_marker_in(dir: &Path, filename: &str) -> Result<()> { fn remove_marker_in(dir: &Path, filename: &str) -> Result<()> {
let path = dir.join(filename); let path = dir.join(filename);
match std::fs::remove_file(&path) { match std::fs::remove_file(&path) {
@ -1486,10 +1682,10 @@ fn remove_marker_in(dir: &Path, filename: &str) -> Result<()> {
/// `harness/`) — both write into a directory owned by the agent, which is /// `harness/`) — both write into a directory owned by the agent, which is
/// precisely why they need hive-priv at all. /// precisely why they need hive-priv at all.
/// ///
/// The file is published by `rename`, so a reader woken by its appearance /// The file is published through a [`StagedFile`], so a reader woken by its
/// cannot catch it empty or half-written: several of these paths have a /// appearance cannot catch it empty or half-written: several of these paths
/// `systemd.path` unit watching them, and one of those triggers on the file /// have a `systemd.path` unit watching them, and one of those triggers on the
/// existing rather than changing. /// file existing rather than changing.
fn write_agent_dir_file( fn write_agent_dir_file(
agent_name: &str, agent_name: &str,
dir: &Path, dir: &Path,
@ -1499,8 +1695,7 @@ fn write_agent_dir_file(
use std::os::fd::AsRawFd as _; use std::os::fd::AsRawFd as _;
use std::os::unix::fs::MetadataExt as _; use std::os::unix::fs::MetadataExt as _;
// The leaf is validated here as well as in `write_state_file_nofollow`: // Checked before `create_dir_all`, so a bad leaf does no filesystem work.
// that call now sees the temp name, which is plain whatever `filename` is.
ensure_plain_filename("write_agent_dir_file", filename)?; ensure_plain_filename("write_agent_dir_file", filename)?;
let state_dir = dir.to_path_buf(); let state_dir = dir.to_path_buf();
@ -1514,24 +1709,24 @@ fn write_agent_dir_file(
std::fs::create_dir_all(&state_dir) std::fs::create_dir_all(&state_dir)
.with_context(|| format!("create state dir {}", state_dir.display()))?; .with_context(|| format!("create state dir {}", state_dir.display()))?;
// Security-critical: refuses a symlink the (state/-owning) agent may have // Security-critical: the (state/-owning) agent may plant a symlink or
// planted at the leaf, so this root-privileged create/write/chmod/chown // hardlink in this dir, and `StagedFile` neither follows nor reuses one,
// can't be redirected at an arbitrary file. See `write_state_file_nofollow`. // so this root-privileged create/write/chmod/chown can't be redirected at
// an arbitrary file.
let path = state_dir.join(filename); let path = state_dir.join(filename);
let tmp_name = partial_name(filename); let dir_fd = open_dir(&state_dir)?;
let tmp_path = state_dir.join(&tmp_name); let staged = StagedFile::write(&dir_fd, &state_dir, filename, content.as_bytes(), 0o600)?;
let file = write_state_file_nofollow(&state_dir, &tmp_name, content)?;
// Chown to the state dir's owner so the agent process can read the file. // Chown to the state dir's owner so the agent process can read the file.
// fchown on the same fd — TOCTOU-immune (the inode the write hit, never a // fchown on the temp's fd — TOCTOU-immune (the inode the write hit, never a
// swapped path). If stat fails (e.g. dir just created, owner is root), the // swapped path). If stat fails (e.g. dir just created, owner is root), the
// file stays root-owned and 0600 — still unreadable by others, just not // file stays root-owned and 0600 — still unreadable by others, just not
// agent-readable. Log a warning so operators can diagnose. // agent-readable. Log a warning so operators can diagnose.
match std::fs::metadata(&state_dir) { match dir_fd.metadata() {
Ok(meta) => { Ok(meta) => {
// SAFETY: `file` is an open, owned fd live for the whole call; // SAFETY: the temp's fd is open and owned by `staged` for the whole
// `fchown` only mutates that inode's uid/gid. // call; `fchown` only mutates that inode's uid/gid.
let rc = unsafe { libc::fchown(file.as_raw_fd(), meta.uid(), meta.gid()) }; let rc = unsafe { libc::fchown(staged.file().as_raw_fd(), meta.uid(), meta.gid()) };
if rc != 0 { if rc != 0 {
let e = std::io::Error::last_os_error(); let e = std::io::Error::last_os_error();
tracing::warn!( tracing::warn!(
@ -1551,12 +1746,9 @@ fn write_agent_dir_file(
} }
} }
// After the chown, never before: the file must never be visible under its // After the chown, never before: the file must never be visible under its
// final name while still root-owned. Leaving the temp behind on failure // final name while still root-owned. A failed publish drops `staged`,
// would also leave a credential readable by nobody but root, so it goes. // which removes the temp and the credential in it.
if let Err(e) = std::fs::rename(&tmp_path, &path) { staged.publish()?;
let _ = std::fs::remove_file(&tmp_path);
return Err(e).with_context(|| format!("publishing {}", path.display()));
}
tracing::info!( tracing::info!(
agent = %agent_name, agent = %agent_name,
@ -2966,6 +3158,7 @@ fn write_nspawn_flags(
load_credentials: &[CredentialMount], load_credentials: &[CredentialMount],
) -> Result<()> { ) -> Result<()> {
use std::fmt::Write as _; use std::fmt::Write as _;
use std::os::unix::fs::MetadataExt as _;
let path = format!("/etc/nixos-containers/{container}.conf"); let path = format!("/etc/nixos-containers/{container}.conf");
let original = std::fs::read_to_string(&path).with_context(|| format!("read {path}"))?; let original = std::fs::read_to_string(&path).with_context(|| format!("read {path}"))?;
let lines: Vec<&str> = original let lines: Vec<&str> = original
@ -3025,7 +3218,15 @@ fn write_nspawn_flags(
} }
let flags_joined = flags.join(" "); let flags_joined = flags.join(" ");
let _ = writeln!(out, "EXTRA_NSPAWN_FLAGS=\"{flags_joined}\""); let _ = writeln!(out, "EXTRA_NSPAWN_FLAGS=\"{flags_joined}\"");
std::fs::write(&path, out).with_context(|| format!("write {path}"))?; // Keeps the owner and mode `nixos-container` created the file with; this
// helper only rewrites its lines.
let meta = std::fs::metadata(&path).with_context(|| format!("stat {path}"))?;
publish_file(
Path::new(&path),
out.as_bytes(),
meta.mode() & 0o7777,
Some((meta.uid(), meta.gid())),
)?;
// DNS marker for the in-container resolver oneshot: the oneshot only // DNS marker for the in-container resolver oneshot: the oneshot only
// rewrites the container's resolv.conf when this marker exists, and the // rewrites the container's resolv.conf when this marker exists, and the
@ -3039,7 +3240,7 @@ fn write_nspawn_flags(
} }
/// Filename of the in-container DNS marker, in the container's own `/etc`. /// Filename of the in-container DNS marker, in the container's own `/etc`.
const BRIDGE_DNS_MARKER: &std::ffi::CStr = c"hyperhive-bridge-dns"; const BRIDGE_DNS_MARKER: &str = "hyperhive-bridge-dns";
/// Write the bridge-DNS marker the `hyperhive-isolated-dns` oneshot keys /// Write the bridge-DNS marker the `hyperhive-isolated-dns` oneshot keys
/// off. The marker file contains just the gateway IP. Always written: /// off. The marker file contains just the gateway IP. Always written:
@ -3054,24 +3255,20 @@ fn write_bridge_dns_marker(container: &str, isolation: &NetworkIsolation) -> Res
/// ///
/// Root inside the container owns its `/etc`, and this write runs as host /// Root inside the container owns its `/etc`, and this write runs as host
/// root, where an absolute symlink planted in the container resolves against /// root, where an absolute symlink planted in the container resolves against
/// the host's `/`. So this opens `etc` with `O_DIRECTORY|O_NOFOLLOW` and the /// the host's `/`. So this opens `etc` with `O_DIRECTORY|O_NOFOLLOW` (a
/// leaf relative to that fd with `O_NOFOLLOW`: a symlink at either step fails /// symlinked `etc` fails with `ELOOP`) and publishes the marker through a
/// the write (`ELOOP`) instead of redirecting it. It also opens the leaf /// [`StagedFile`] relative to that fd: whatever the container planted at the
/// `O_NONBLOCK` and requires a regular file, so a FIFO cannot hang the helper /// leaf, symlink or FIFO or device node, is replaced by the rename, never
/// and a device node cannot take the write. /// written through or opened.
/// ///
/// On a fresh install the container's `/etc` may not exist yet (nixos-container /// On a fresh install the container's `/etc` may not exist yet (nixos-container
/// materialises the rootfs on the first start), so this creates it first; the /// materialises the rootfs on the first start), so this creates it first; the
/// dir and the marker persist through that start. Mode 0644: the gateway IP /// dir and the marker persist through that start. Mode 0644: the gateway IP
/// is not a secret, and 0644 matches what the default 0022 umask gave the /// is not a secret.
/// earlier plain write.
fn write_bridge_dns_marker_in(rootfs: &Path, gateway_ip: &str) -> Result<()> { fn write_bridge_dns_marker_in(rootfs: &Path, gateway_ip: &str) -> Result<()> {
use std::io::Write as _; use std::os::unix::fs::OpenOptionsExt as _;
use std::os::unix::ffi::OsStrExt as _;
use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _};
let etc_path = rootfs.join("etc"); let etc_path = rootfs.join("etc");
let path = etc_path.join(std::ffi::OsStr::from_bytes(BRIDGE_DNS_MARKER.to_bytes()));
// The rootfs dir is the container's `/`, which the container cannot // The rootfs dir is the container's `/`, which the container cannot
// replace, so creating it by path is safe. // replace, so creating it by path is safe.
std::fs::create_dir_all(rootfs) std::fs::create_dir_all(rootfs)
@ -3102,41 +3299,15 @@ fn write_bridge_dns_marker_in(rootfs: &Path, gateway_ip: &str) -> Result<()> {
.with_context(|| format!("open (no-follow) {}", etc_path.display())); .with_context(|| format!("open (no-follow) {}", etc_path.display()));
} }
// SAFETY: `etc_fd` is a fresh, valid fd that nothing else owns. // SAFETY: `etc_fd` is a fresh, valid fd that nothing else owns.
let etc = unsafe { OwnedFd::from_raw_fd(etc_fd) }; let etc = std::fs::File::from(unsafe { OwnedFd::from_raw_fd(etc_fd) });
// SAFETY: `etc` is an open directory fd and the name a NUL-terminated StagedFile::write(
// literal; this checks the result before wrapping it. &etc,
let leaf_fd = unsafe { &etc_path,
libc::openat( BRIDGE_DNS_MARKER,
etc.as_raw_fd(), format!("{gateway_ip}\n").as_bytes(),
BRIDGE_DNS_MARKER.as_ptr(), 0o644,
libc::O_WRONLY )?
| libc::O_CREAT .publish()
| libc::O_TRUNC
| libc::O_NOFOLLOW
| libc::O_NONBLOCK
| libc::O_CLOEXEC,
0o644 as libc::c_uint,
)
};
if leaf_fd < 0 {
return Err(std::io::Error::last_os_error())
.with_context(|| format!("open (no-follow) {}", path.display()));
}
// SAFETY: `leaf_fd` is a fresh, valid fd that nothing else owns.
let mut file = std::fs::File::from(unsafe { OwnedFd::from_raw_fd(leaf_fd) });
let is_file = file
.metadata()
.with_context(|| format!("stat {}", path.display()))?
.file_type()
.is_file();
if !is_file {
bail!("{} is not a regular file", path.display());
}
file.write_all(format!("{gateway_ip}\n").as_bytes())
.with_context(|| format!("write bridge-DNS marker {}", path.display()))?;
file.set_permissions(std::fs::Permissions::from_mode(0o644))
.with_context(|| format!("chmod 644 {}", path.display()))?;
Ok(())
} }
/// `SyncAgentTmpfiles` — write `/etc/tmpfiles.d/hyperhive-agents.conf` for /// `SyncAgentTmpfiles` — write `/etc/tmpfiles.d/hyperhive-agents.conf` for
@ -3232,12 +3403,9 @@ async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<(String, S
} }
let content = agent_tmpfiles_content(agents); let content = agent_tmpfiles_content(agents);
// Atomic write: write to a tmp file then rename so a concurrent reader // Overlapping syncs each stage their own temp, so the last rename wins
// always sees a complete file. // with one complete roster rather than a mix of two.
let tmp = format!("{TMPFILES_PATH}.tmp"); publish_file(Path::new(TMPFILES_PATH), content.as_bytes(), 0o644, None)?;
std::fs::write(&tmp, &content).with_context(|| format!("write {tmp}"))?;
std::fs::rename(&tmp, TMPFILES_PATH)
.with_context(|| format!("rename {TMPFILES_PATH}.tmp -> {TMPFILES_PATH}"))?;
tracing::info!(agents = agents.len(), "tmpfiles.d: wrote {TMPFILES_PATH}"); tracing::info!(agents = agents.len(), "tmpfiles.d: wrote {TMPFILES_PATH}");
// Apply immediately so dirs exist on the running host, not just after next boot. // Apply immediately so dirs exist on the running host, not just after next boot.
@ -3259,13 +3427,13 @@ async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<(String, S
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::{ use super::{
AgentTmpfilesEntry, BindMount, OwnedFd, PAUSED_MARKER_FILE, PrivRequest, AgentTmpfilesEntry, BindMount, BoundedRun, OwnedFd, PAUSED_MARKER_FILE, PrivRequest,
agent_tmpfiles_content, check_fd_agreement, clear_runner_credentials, StagedFile, agent_tmpfiles_content, check_fd_agreement, clear_runner_credentials,
contains_secret_shaped_run, describe_forge_admin, ensure_plain_filename, git_overlay_flags, contains_secret_shaped_run, describe_forge_admin, ensure_plain_filename, git_overlay_flags,
limits_dropin_body, matrix_token_filename, open_export_dest, partial_name, limits_dropin_body, matrix_token_filename, open_dir, open_export_dest, partial_name,
redact_secret_line, remove_marker_in, single_output_path, toplevel_attr, publish_file, redact_secret_line, remove_marker_in, run_bounded, single_output_path,
validate_account_name, validate_credential_name, validate_snapshot_name, toplevel_attr, validate_account_name, validate_credential_name, validate_snapshot_name,
write_agent_dir_file, write_bridge_dns_marker_in, write_state_file_nofollow, write_agent_dir_file, write_bridge_dns_marker_in,
}; };
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::atomic::{AtomicU32, Ordering};
@ -3669,17 +3837,16 @@ mod tests {
#[test] #[test]
fn rejects_non_plain_filenames() { fn rejects_non_plain_filenames() {
let dir = scratch(); let dir = scratch();
let dir_fd = open_dir(&dir).unwrap();
for bad in ["", ".", "..", "a/b", "/etc/passwd", "../escape", "sub/tok"] { for bad in ["", ".", "..", "a/b", "/etc/passwd", "../escape", "sub/tok"] {
assert!( assert!(
write_state_file_nofollow(&dir, bad, "x").is_err(), StagedFile::write(&dir_fd, &dir, bad, b"x", 0o600).is_err(),
"must reject filename {bad:?}" "must reject filename {bad:?}"
); );
// The wrapper needs its own check: the only name it hands to // The wrapper's own check runs before its `create_dir_all`.
// `write_state_file_nofollow` is the temp, which is plain whatever // Pointing it at a directory that does not exist yet is what makes
// the caller passed. Pointing it at a directory that does not exist // this arm bite: the property worth pinning is that a bad leaf
// yet is what makes this arm bite -- a bad leaf fails eventually // fails BEFORE any root-privileged filesystem work.
// either way, at the rename, so the property worth pinning is that
// it fails BEFORE any root-privileged filesystem work.
let absent = dir.join("never-created"); let absent = dir.join("never-created");
assert!( assert!(
write_agent_dir_file("a", &absent, bad, "x").is_err(), write_agent_dir_file("a", &absent, bad, "x").is_err(),
@ -3786,65 +3953,143 @@ mod tests {
"must fail AT the publish -- failing earlier leaves no temp to \ "must fail AT the publish -- failing earlier leaves no temp to \
clean up and makes the next assertion vacuous. got: {err:#}" clean up and makes the next assertion vacuous. got: {err:#}"
); );
assert!( assert_eq!(
!dir.join(partial_name("forge-token")).exists(), entries(&dir),
["forge-token"],
"temp still holds the secret after a failed publish" "temp still holds the secret after a failed publish"
); );
std::fs::remove_dir_all(&dir).ok(); std::fs::remove_dir_all(&dir).ok();
} }
/// Names in `dir`, sorted — the residue check every publish test ends on.
fn entries(dir: &PathBuf) -> Vec<String> {
let mut v: Vec<String> = std::fs::read_dir(dir)
.unwrap()
.map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
.collect();
v.sort();
v
}
/// The agent plants a symlink where the token goes. The rename replaces
/// the link; the root-privileged write never reaches its target.
#[test] #[test]
fn refuses_symlink_leaf_and_leaves_target_untouched() { fn a_symlink_at_the_name_is_replaced_not_followed() {
let dir = scratch(); let dir = scratch();
let target = dir.join("target"); let target = dir.join("target");
std::fs::write(&target, "original").unwrap(); std::fs::write(&target, "original").unwrap();
// Agent plants a symlink where the token would be written.
std::os::unix::fs::symlink(&target, dir.join("forge-token")).unwrap(); std::os::unix::fs::symlink(&target, dir.join("forge-token")).unwrap();
let res = write_state_file_nofollow(&dir, "forge-token", "PWNED"); write_agent_dir_file("a", &dir, "forge-token", "SECRET").unwrap();
assert!(res.is_err(), "O_NOFOLLOW must refuse a symlink leaf");
// The root-privileged write must NOT have followed the link.
assert_eq!( assert_eq!(
std::fs::read_to_string(&target).unwrap(), std::fs::read_to_string(&target).unwrap(),
"original", "original",
"symlink target must be untouched" "symlink target must be untouched"
); );
let meta = std::fs::symlink_metadata(dir.join("forge-token")).unwrap();
assert!(meta.file_type().is_file(), "the link must be replaced");
assert_eq!(
std::fs::read_to_string(dir.join("forge-token")).unwrap(),
"SECRET"
);
std::fs::remove_dir_all(&dir).ok(); std::fs::remove_dir_all(&dir).ok();
} }
/// Any failure between staging and publishing drops the `StagedFile`.
/// Until the rename a reader sees only the old file, and afterwards the
/// old file is still whole and the temp is gone.
#[test] #[test]
fn writes_plain_file_0600() { fn a_failed_write_leaves_the_old_file_intact() {
use std::os::unix::fs::PermissionsExt as _;
let dir = scratch(); let dir = scratch();
write_state_file_nofollow(&dir, "forge-token", "secret").unwrap(); let dest = dir.join("hyperhive-limits.conf");
let path = dir.join("forge-token"); std::fs::write(&dest, "OLD").unwrap();
assert_eq!(std::fs::read_to_string(&path).unwrap(), "secret"); let dir_fd = open_dir(&dir).unwrap();
let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600, "token file must be 0600"); let staged =
// Overwrite truncates cleanly (O_TRUNC), no residue. StagedFile::write(&dir_fd, &dir, "hyperhive-limits.conf", b"NEW", 0o644).unwrap();
write_state_file_nofollow(&dir, "forge-token", "new").unwrap(); assert_eq!(std::fs::read_to_string(&dest).unwrap(), "OLD");
assert_eq!(std::fs::read_to_string(&path).unwrap(), "new"); assert_eq!(
entries(&dir).len(),
2,
"control: the temp exists while staged"
);
drop(staged);
assert_eq!(std::fs::read_to_string(&dest).unwrap(), "OLD");
assert_eq!(entries(&dir), ["hyperhive-limits.conf"], "temp left behind");
std::fs::remove_dir_all(&dir).ok(); std::fs::remove_dir_all(&dir).ok();
} }
/// The container plants a symlink where the bridge-DNS marker goes. /// Two overlapping writes of one file each own a temp, so whichever
/// publishes last lands whole — never the other's bytes, never a mix.
#[test] #[test]
fn bridge_dns_marker_refuses_symlink_leaf() { fn overlapping_writes_of_one_file_never_share_a_temp() {
let dir = scratch();
let dir_fd = open_dir(&dir).unwrap();
let name = "hyperhive-agents.conf";
let first = StagedFile::write(&dir_fd, &dir, name, b"roster A\n", 0o644).unwrap();
let second = StagedFile::write(&dir_fd, &dir, name, b"roster B, longer\n", 0o644).unwrap();
second.publish().unwrap();
first.publish().unwrap();
assert_eq!(
std::fs::read_to_string(dir.join(name)).unwrap(),
"roster A\n"
);
assert_eq!(entries(&dir), [name], "temp left behind");
std::fs::remove_dir_all(&dir).ok();
}
/// The mode lands exactly as asked, whatever the umask would have made
/// of it (0666 under the usual 022 would otherwise come out 0644), and
/// the requested owner is on the file.
#[test]
fn publish_file_applies_mode_and_owner() {
use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _};
let dir = scratch();
let path = dir.join("c.conf");
let owner = std::fs::metadata(&dir).unwrap();
publish_file(&path, b"x\n", 0o666, Some((owner.uid(), owner.gid()))).unwrap();
let meta = std::fs::metadata(&path).unwrap();
assert_eq!(meta.permissions().mode() & 0o7777, 0o666);
assert_eq!((meta.uid(), meta.gid()), (owner.uid(), owner.gid()));
publish_file(&path, b"y\n", 0o600, None).unwrap();
let meta = std::fs::metadata(&path).unwrap();
assert_eq!(meta.permissions().mode() & 0o7777, 0o600);
assert_eq!(std::fs::read_to_string(&path).unwrap(), "y\n");
assert_eq!(entries(&dir), ["c.conf"], "temp left behind");
std::fs::remove_dir_all(&dir).ok();
}
/// The container plants a symlink where the bridge-DNS marker goes; the
/// rename replaces it without writing through it.
#[test]
fn bridge_dns_marker_replaces_symlink_leaf() {
let dir = scratch(); let dir = scratch();
let rootfs = dir.join("rootfs"); let rootfs = dir.join("rootfs");
std::fs::create_dir_all(rootfs.join("etc")).unwrap(); std::fs::create_dir_all(rootfs.join("etc")).unwrap();
let target = dir.join("target"); let target = dir.join("target");
std::fs::write(&target, "original").unwrap(); std::fs::write(&target, "original").unwrap();
std::os::unix::fs::symlink(&target, rootfs.join("etc/hyperhive-bridge-dns")).unwrap(); let leaf = rootfs.join("etc/hyperhive-bridge-dns");
std::os::unix::fs::symlink(&target, &leaf).unwrap();
let res = write_bridge_dns_marker_in(&rootfs, "10.0.0.1"); write_bridge_dns_marker_in(&rootfs, "10.0.0.1").unwrap();
assert!(res.is_err(), "O_NOFOLLOW must refuse a symlink leaf");
assert_eq!( assert_eq!(
std::fs::read_to_string(&target).unwrap(), std::fs::read_to_string(&target).unwrap(),
"original", "original",
"symlink target must be untouched" "symlink target must be untouched"
); );
assert!(
std::fs::symlink_metadata(&leaf)
.unwrap()
.file_type()
.is_file()
);
assert_eq!(std::fs::read_to_string(&leaf).unwrap(), "10.0.0.1\n");
std::fs::remove_dir_all(&dir).ok(); std::fs::remove_dir_all(&dir).ok();
} }
@ -3871,9 +4116,10 @@ mod tests {
std::fs::remove_dir_all(&dir).ok(); std::fs::remove_dir_all(&dir).ok();
} }
/// A FIFO at the leaf fails the write rather than blocking the helper. /// A FIFO at the leaf is replaced, never opened, so it cannot block the
/// helper.
#[test] #[test]
fn bridge_dns_marker_refuses_fifo_leaf() { fn bridge_dns_marker_replaces_fifo_leaf() {
use std::os::unix::ffi::OsStrExt as _; use std::os::unix::ffi::OsStrExt as _;
let dir = scratch(); let dir = scratch();
let rootfs = dir.join("rootfs"); let rootfs = dir.join("rootfs");
@ -3883,13 +4129,19 @@ mod tests {
// SAFETY: `c_fifo` is a valid NUL-terminated path. // SAFETY: `c_fifo` is a valid NUL-terminated path.
assert_eq!(unsafe { libc::mkfifo(c_fifo.as_ptr(), 0o644) }, 0); assert_eq!(unsafe { libc::mkfifo(c_fifo.as_ptr(), 0o644) }, 0);
let res = write_bridge_dns_marker_in(&rootfs, "10.0.0.1"); write_bridge_dns_marker_in(&rootfs, "10.0.0.1").unwrap();
assert!(res.is_err(), "a FIFO leaf must be refused"); assert!(
std::fs::symlink_metadata(&fifo)
.unwrap()
.file_type()
.is_file()
);
assert_eq!(std::fs::read_to_string(&fifo).unwrap(), "10.0.0.1\n");
std::fs::remove_dir_all(&dir).ok(); std::fs::remove_dir_all(&dir).ok();
} }
/// The control: a plain rootfs with no `etc` yet (a fresh install) gets /// The control: a plain rootfs with no `etc` yet (a fresh install) gets
/// the dir and a 0644 marker, and a second write truncates. /// the dir and a 0644 marker, and a second write replaces it.
#[test] #[test]
fn bridge_dns_marker_written_into_plain_etc() { fn bridge_dns_marker_written_into_plain_etc() {
use std::os::unix::fs::PermissionsExt as _; use std::os::unix::fs::PermissionsExt as _;
@ -3908,6 +4160,74 @@ mod tests {
std::fs::remove_dir_all(&dir).ok(); std::fs::remove_dir_all(&dir).ok();
} }
/// Whether `pid` is gone, or only a zombie awaiting its reaper: either
/// way it is no longer running.
fn is_dead(pid: &str) -> bool {
match std::fs::read_to_string(format!("/proc/{pid}/stat")) {
Err(_) => true,
// The state letter follows the parenthesised command name.
Ok(stat) => stat
.rsplit_once(") ")
.is_some_and(|(_, rest)| rest.starts_with('Z')),
}
}
/// A child still running at the limit comes back `TimedOut`, and the kill
/// reaches its whole process group: the backgrounded grandchild dies too,
/// not just the shell that started it.
#[tokio::test]
async fn a_build_past_its_limit_is_killed_with_its_group() {
use std::time::{Duration, Instant};
let dir = scratch();
let pidfile = dir.join("grandchild");
let mut cmd = tokio::process::Command::new("sh");
cmd.arg("-c")
.arg(format!("sleep 600 & echo $! > {}; wait", pidfile.display()));
let started = Instant::now();
let run = run_bounded(cmd, Duration::from_secs(2), None)
.await
.unwrap();
assert!(matches!(run, BoundedRun::TimedOut), "must time out");
assert!(
started.elapsed() < Duration::from_mins(1),
"the limit did not bound the run"
);
let pid = std::fs::read_to_string(&pidfile).unwrap();
let pid = pid.trim();
let deadline = Instant::now() + Duration::from_secs(5);
while !is_dead(pid) && Instant::now() < deadline {
tokio::time::sleep(Duration::from_millis(50)).await;
}
assert!(is_dead(pid), "grandchild {pid} outlived the timeout");
std::fs::remove_dir_all(&dir).ok();
}
/// The control: a child that exits inside its limit is `Exited`, with its
/// status and both streams kept apart.
#[tokio::test]
async fn a_build_inside_its_limit_returns_its_output() {
let mut cmd = tokio::process::Command::new("sh");
cmd.arg("-c")
.arg("echo progress >&2; echo /nix/store/abc-toplevel");
match run_bounded(cmd, std::time::Duration::from_mins(1), None)
.await
.unwrap()
{
BoundedRun::Exited {
status,
stdout,
stderr,
} => {
assert!(status.success());
assert_eq!(stdout, "/nix/store/abc-toplevel\n");
assert_eq!(stderr, "progress");
}
BoundedRun::TimedOut => panic!("a fast child must not time out"),
}
}
/// The runner-credential clear, in all three states that matter. The /// The runner-credential clear, in all three states that matter. The
/// PRESENCE arm is the load-bearing one: an implementation that did nothing /// PRESENCE arm is the load-bearing one: an implementation that did nothing
/// at all would pass the "absent is fine" arm perfectly, and the whole point /// at all would pass the "absent is fine" arm perfectly, and the whole point
@ -3972,7 +4292,7 @@ mod tests {
let path = dir.join(PAUSED_MARKER_FILE); let path = dir.join(PAUSED_MARKER_FILE);
for _ in 0..2 { for _ in 0..2 {
write_state_file_nofollow(&dir, PAUSED_MARKER_FILE, "").unwrap(); write_agent_dir_file("a", &dir, PAUSED_MARKER_FILE, "").unwrap();
assert!(path.exists(), "marker must exist after pause"); assert!(path.exists(), "marker must exist after pause");
assert_eq!(std::fs::read_to_string(&path).unwrap(), ""); assert_eq!(std::fs::read_to_string(&path).unwrap(), "");
} }