`agent_uid_gid` returns `None` for all 13 agents on every sync, and the
tmpfiles caller answers that `None` by writing `d /run/hive-agent/<name>
0777 root root` instead of `0751 <uid> <gid>` — a world-writable socket
dir, which docs/trust-boundary/boundary.md spells out as letting anything
that can reach the path unlink an agent's socket and bind its own.
Which failure fires could not be determined, because the read used
`.ok()?` and collapsed every io::Error into the same `None` a missing
user produces. None of the three causes the doc comment enumerated (not
built yet, unparseable, missing user) fits 13 long-lived containers whose
agent user demonstrably exists — from inside one:
srw-rw-rw- atlas atlas /run/hive-agent/atlas/agent.sock
So the live cause was outside the documented set and unidentifiable. Both
`None` arms now log, with the path and the error.
Splitting the pure parser out to make it testable surfaced a second,
narrower bug. The old scan used `?` on the field reads, and those are
only reached once the name matches — so an unusable row *for the wanted
user* returned `None` from the whole function instead of skipping, hiding
a usable entry below it. (Rows for other users were always skipped fine:
`split(':')` always yields at least one item, so the first `?` could not
fire.) It now skips unusable rows and keeps looking.
Does not pre-empt #3047, which removes the lookup entirely and stays
blocked on #3998; this only makes the lookup honest about failing while
it exists.
Closes #4197.
328 lines
13 KiB
Rust
328 lines
13 KiB
Rust
//! Unit tests for the lifecycle module (moved verbatim from the old
|
|
//! single-file `lifecycle.rs` `#[cfg(test)]` block).
|
|
|
|
use super::*;
|
|
|
|
/// Regression test: `setup_proposed` must seed both agent.nix and flake.nix
|
|
/// in the initial commit. Before commit 5b5a93e flake.nix was missing from
|
|
/// the scaffold, requiring manual creation (seen with the damocles agent).
|
|
///
|
|
/// Exercises the template arm: there is no forge to clone from in a test
|
|
/// (`forge::is_present` needs the priv socket), so `setup_proposed` falls
|
|
/// through to `seed_template` — which is the arm this asserts about.
|
|
#[tokio::test]
|
|
async fn setup_proposed_seeds_flake_nix() {
|
|
let dir = tempfile::tempdir().expect("tempdir");
|
|
let proposed = dir.path().join("proposed");
|
|
setup_proposed(&proposed, "test-agent")
|
|
.await
|
|
.expect("setup_proposed");
|
|
|
|
// Both files must exist on disk.
|
|
assert!(proposed.join("agent.nix").exists(), "agent.nix missing");
|
|
assert!(proposed.join("flake.nix").exists(), "flake.nix missing");
|
|
|
|
// flake.nix must export nixosModules.default (the meta-flake contract).
|
|
let flake = std::fs::read_to_string(proposed.join("flake.nix")).unwrap();
|
|
assert!(
|
|
flake.contains("nixosModules.default"),
|
|
"flake.nix does not export nixosModules.default"
|
|
);
|
|
|
|
// Both files must be tracked in the initial git commit.
|
|
let out = git_command()
|
|
.current_dir(&proposed)
|
|
.args(["show", "--name-only", "--format=", "HEAD"])
|
|
.output()
|
|
.await
|
|
.expect("git show");
|
|
let tracked = String::from_utf8_lossy(&out.stdout);
|
|
assert!(tracked.contains("agent.nix"), "agent.nix not committed");
|
|
assert!(tracked.contains("flake.nix"), "flake.nix not committed");
|
|
}
|
|
|
|
#[test]
|
|
fn bridge_gateway_ip_extracts_verbatim_address() {
|
|
// HIVE_NETWORK_SUBNET carries the bridge IP verbatim, not the
|
|
// canonical network — the gateway is the address before the `/`.
|
|
assert_eq!(
|
|
bridge_gateway_ip("10.42.0.1/24").as_deref(),
|
|
Some("10.42.0.1")
|
|
);
|
|
// Non-`.1` operator override: the gateway is wherever the bridge is.
|
|
assert_eq!(
|
|
bridge_gateway_ip("10.42.0.254/24").as_deref(),
|
|
Some("10.42.0.254")
|
|
);
|
|
assert_eq!(
|
|
bridge_gateway_ip("172.30.0.1/16").as_deref(),
|
|
Some("172.30.0.1")
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn bridge_gateway_ip_rejects_bad_input() {
|
|
assert!(bridge_gateway_ip("notanip/24").is_none());
|
|
assert!(bridge_gateway_ip("10.42.0.1").is_none()); // no prefix
|
|
assert!(bridge_gateway_ip("10.42.0.1/33").is_none()); // prefix > 32
|
|
assert!(bridge_gateway_ip("10.42.0.999/24").is_none()); // octet > 255
|
|
assert!(bridge_gateway_ip("10.42.0/24").is_none()); // 3 octets
|
|
}
|
|
|
|
/// The presence control for the two rejection tests below: with both
|
|
/// variables set and well-formed, the settings are built. Without this,
|
|
/// a `network_isolation_from_vars` that rejected *everything* would pass
|
|
/// every absence assertion and look like a working guard.
|
|
#[test]
|
|
fn network_isolation_accepts_a_well_formed_pair() {
|
|
let iso = network_isolation_from_vars(Some("hive0"), Some("10.42.0.1/24"))
|
|
.expect("well-formed bridge + subnet must be accepted");
|
|
assert_eq!(iso.bridge, "hive0");
|
|
// The gateway is the verbatim bridge address, prefix stripped.
|
|
assert_eq!(iso.gateway_ip, "10.42.0.1");
|
|
}
|
|
|
|
/// A missing or empty variable is fatal, not a fallback to the host
|
|
/// netns. Empty is tested alongside unset because `std::env::var` on a
|
|
/// variable set to `""` returns `Ok("")`, so treating only `None` as
|
|
/// missing would let an empty value through.
|
|
#[test]
|
|
fn network_isolation_rejects_missing_or_empty_vars() {
|
|
assert!(network_isolation_from_vars(None, Some("10.42.0.1/24")).is_err());
|
|
assert!(network_isolation_from_vars(Some("hive0"), None).is_err());
|
|
assert!(network_isolation_from_vars(None, None).is_err());
|
|
assert!(network_isolation_from_vars(Some(""), Some("10.42.0.1/24")).is_err());
|
|
assert!(network_isolation_from_vars(Some("hive0"), Some("")).is_err());
|
|
}
|
|
|
|
/// A malformed subnet is fatal too. Previously this logged a warning and
|
|
/// silently produced a container on the host netns — a dropped security
|
|
/// boundary with nothing in the journal saying so.
|
|
#[test]
|
|
fn network_isolation_rejects_a_malformed_subnet() {
|
|
assert!(network_isolation_from_vars(Some("hive0"), Some("notanip/24")).is_err());
|
|
assert!(network_isolation_from_vars(Some("hive0"), Some("10.42.0.1")).is_err());
|
|
assert!(network_isolation_from_vars(Some("hive0"), Some("10.42.0.1/33")).is_err());
|
|
assert!(network_isolation_from_vars(Some("hive0"), Some("10.42.0.999/24")).is_err());
|
|
}
|
|
|
|
/// The error has to name the variable an operator must fix — these are
|
|
/// read at startup, so the message is the whole diagnostic.
|
|
#[test]
|
|
fn network_isolation_errors_name_the_offending_variable() {
|
|
let e = network_isolation_from_vars(None, Some("10.42.0.1/24")).unwrap_err();
|
|
assert!(
|
|
format!("{e:#}").contains("HIVE_NETWORK_BRIDGE"),
|
|
"bridge error must name the variable, got: {e:#}"
|
|
);
|
|
let e = network_isolation_from_vars(Some("hive0"), None).unwrap_err();
|
|
assert!(
|
|
format!("{e:#}").contains("HIVE_NETWORK_SUBNET"),
|
|
"subnet error must name the variable, got: {e:#}"
|
|
);
|
|
let e = network_isolation_from_vars(Some("hive0"), Some("nope/24")).unwrap_err();
|
|
let msg = format!("{e:#}");
|
|
assert!(
|
|
msg.contains("HIVE_NETWORK_SUBNET") && msg.contains("nope/24"),
|
|
"malformed-subnet error must name the variable and the bad value, got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// `setup_proposed` is idempotent: calling it on an existing repo is a
|
|
/// no-op (the fresh guard skips all writes).
|
|
#[tokio::test]
|
|
async fn setup_proposed_idempotent() {
|
|
let dir = tempfile::tempdir().expect("tempdir");
|
|
let proposed = dir.path().join("proposed");
|
|
setup_proposed(&proposed, "test-agent")
|
|
.await
|
|
.expect("first call");
|
|
// Second call must not error even though .git already exists.
|
|
setup_proposed(&proposed, "test-agent")
|
|
.await
|
|
.expect("second call");
|
|
// Still one commit.
|
|
let out = git_command()
|
|
.current_dir(&proposed)
|
|
.args(["rev-list", "--count", "HEAD"])
|
|
.output()
|
|
.await
|
|
.expect("git rev-list");
|
|
let count = String::from_utf8_lossy(&out.stdout).trim().to_owned();
|
|
assert_eq!(
|
|
count, "1",
|
|
"expected exactly one commit after idempotent call"
|
|
);
|
|
}
|
|
|
|
/// Build a two-commit repo and return `(dir, repo_path, first, second)`.
|
|
async fn two_commit_repo() -> (tempfile::TempDir, std::path::PathBuf, String, String) {
|
|
let dir = tempfile::tempdir().expect("tempdir");
|
|
let repo = dir.path().join("proposed");
|
|
setup_proposed(&repo, "test-agent")
|
|
.await
|
|
.expect("setup_proposed");
|
|
let first = git_rev_parse(&repo, "HEAD").await.expect("rev-parse first");
|
|
|
|
std::fs::write(repo.join("second.txt"), "second").expect("write second.txt");
|
|
git(&repo, &["add", "second.txt"]).await.expect("git add");
|
|
git(
|
|
&repo,
|
|
&[
|
|
"-c",
|
|
"user.name=test",
|
|
"-c",
|
|
"user.email=test@example.com",
|
|
"commit",
|
|
"-m",
|
|
"second",
|
|
],
|
|
)
|
|
.await
|
|
.expect("git commit");
|
|
let second = git_rev_parse(&repo, "HEAD")
|
|
.await
|
|
.expect("rev-parse second");
|
|
|
|
assert_ne!(first, second, "second commit did not advance HEAD");
|
|
(dir, repo, first, second)
|
|
}
|
|
|
|
/// `git_is_ancestor` answers reachability in both directions. This is the
|
|
/// check that decides whether moving a branch is a real fast-forward or a
|
|
/// rewind that discards commits.
|
|
#[tokio::test]
|
|
async fn is_ancestor_distinguishes_direction() {
|
|
let (_dir, repo, first, second) = two_commit_repo().await;
|
|
|
|
assert!(
|
|
git_is_ancestor(&repo, &first, &second)
|
|
.await
|
|
.expect("ancestor check forward"),
|
|
"first must be an ancestor of second"
|
|
);
|
|
assert!(
|
|
!git_is_ancestor(&repo, &second, &first)
|
|
.await
|
|
.expect("ancestor check reverse"),
|
|
"second must NOT be an ancestor of first"
|
|
);
|
|
}
|
|
|
|
/// Regression test for the deploy path that ate a committed agent config:
|
|
/// `git_update_ref_cas` must refuse to move a ref whose current value is not
|
|
/// the expected one, and must leave the ref untouched when it refuses. A bare
|
|
/// `update-ref` accepts that move and silently drops everything in between.
|
|
#[tokio::test]
|
|
async fn update_ref_cas_refuses_stale_expectation() {
|
|
let (_dir, repo, first, second) = two_commit_repo().await;
|
|
|
|
// Park a ref at `first`, then CAS it forward with the correct old value.
|
|
git_update_ref(&repo, "refs/heads/cas", &first)
|
|
.await
|
|
.expect("plant ref");
|
|
git_update_ref_cas(&repo, "refs/heads/cas", &second, &first)
|
|
.await
|
|
.expect("CAS with the correct old value must succeed");
|
|
assert_eq!(
|
|
git_rev_parse(&repo, "refs/heads/cas").await.unwrap(),
|
|
second
|
|
);
|
|
|
|
// Now retry with the SAME (now stale) expectation, as a racing deploy
|
|
// holding a pre-merge sha would: it must fail rather than rewind.
|
|
assert!(
|
|
git_update_ref_cas(&repo, "refs/heads/cas", &first, &first)
|
|
.await
|
|
.is_err(),
|
|
"CAS accepted a stale old value"
|
|
);
|
|
assert_eq!(
|
|
git_rev_parse(&repo, "refs/heads/cas").await.unwrap(),
|
|
second,
|
|
"ref moved even though the CAS failed"
|
|
);
|
|
}
|
|
|
|
/// `systemctl is-active` prints the state with a trailing newline, which is
|
|
/// the whole reason this parser trims rather than comparing raw.
|
|
#[test]
|
|
fn unit_state_reads_what_is_active_prints() {
|
|
assert_eq!(UnitState::from_is_active("active\n"), UnitState::Active);
|
|
assert_eq!(UnitState::from_is_active("failed\n"), UnitState::Failed);
|
|
assert_eq!(UnitState::from_is_active("inactive\n"), UnitState::Other);
|
|
// No trailing newline, in case the capture ever changes shape.
|
|
assert_eq!(UnitState::from_is_active("failed"), UnitState::Failed);
|
|
}
|
|
|
|
/// The asymmetry that matters: an unrecognised state must fall to `Other`,
|
|
/// never to `Failed`.
|
|
///
|
|
/// `Failed` is what the dashboard will render as *this agent gave up*, so a
|
|
/// wrong `Failed` invents an incident, while a wrong `Other` merely fails to
|
|
/// distinguish one from a deliberate stop — the behaviour we have today.
|
|
/// Guessing in the safe direction is the property, not an implementation
|
|
/// detail of the `match`.
|
|
#[test]
|
|
fn an_unknown_state_is_never_reported_as_failed() {
|
|
for s in [
|
|
"activating",
|
|
"deactivating",
|
|
"reloading",
|
|
"maintenance",
|
|
"unknown",
|
|
"",
|
|
"Failed", // capitalised: systemd prints lowercase, so this is not a state
|
|
"failed*", // `is-active` never emits this; a substring match would take it
|
|
] {
|
|
assert_eq!(
|
|
UnitState::from_is_active(s),
|
|
UnitState::Other,
|
|
"{s:?} must not be read as a give-up"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// A malformed row for the wanted user must not end the scan.
|
|
///
|
|
/// The old implementation used `?` on the field reads, which are only reached
|
|
/// once the name matches — so a short or unparseable row *for this agent*
|
|
/// returned `None` from the whole function and hid a usable entry below it.
|
|
/// The caller answers that `None` by declaring the socket dir `0777`.
|
|
#[test]
|
|
fn passwd_parse_keeps_scanning_past_a_malformed_row_for_the_same_user() {
|
|
// The two `atlas` rows above the real one are what matters: a malformed
|
|
// row for the SAME user is what used to end the scan, so the usable entry
|
|
// below it was never reached. Rows for other users were always skipped
|
|
// fine. Both unusable shapes are here because they leave the parser by
|
|
// different arms — missing fields, and fields that will not parse — and a
|
|
// mutation run showed the second arm untested when only the first was.
|
|
let body = "\
|
|
root:x:0:0:System administrator:/root:/bin/sh
|
|
# a comment line, not a passwd entry
|
|
atlas:x
|
|
atlas:x:notanumber:994::/:/bin/sh
|
|
atlas:x:1000:994:atlas:/home/atlas:/bin/sh
|
|
";
|
|
assert_eq!(parse_passwd_uid_gid(body, "atlas"), Some((1000, 994)));
|
|
// Control: the same body must NOT answer for a user it does not carry,
|
|
// or the assertion above is satisfied by any parse at all.
|
|
assert_eq!(parse_passwd_uid_gid(body, "nobody-here"), None);
|
|
}
|
|
|
|
/// A matching name with unusable ids is not a match. Without this the parser
|
|
/// could return a partly-parsed row and the caller would chown to it.
|
|
#[test]
|
|
fn passwd_parse_rejects_unusable_ids() {
|
|
assert_eq!(
|
|
parse_passwd_uid_gid("atlas:x:notanumber:994::/:/bin/sh", "atlas"),
|
|
None
|
|
);
|
|
assert_eq!(parse_passwd_uid_gid("atlas:x:1000", "atlas"), None);
|
|
assert_eq!(parse_passwd_uid_gid("", "atlas"), None);
|
|
// Presence control for the three absences above.
|
|
assert_eq!(
|
|
parse_passwd_uid_gid("atlas:x:1000:994::/:/bin/sh", "atlas"),
|
|
Some((1000, 994))
|
|
);
|
|
}
|