test(#2862): cover both fd/op mismatch branches in hive-priv
check_fd_agreement is the guard that keeps a descriptor and the request it arrived with in agreement, and both of its rejections were untested. An fd-taking op with no descriptor must not fall back to anything: a temp file or the response socket would send an agent's state somewhere the caller never asked for. The mirror case matters for a different reason -- returning the error is what drops the OwnedFd and closes it, so ignoring a stray descriptor instead would leak one per bad request in a long-lived root process. The third test pins both agreeing combinations, so the check is rejecting mismatches rather than descriptors in general. Descriptors are real /dev/null handles so the closing drop is genuinely exercised.
This commit is contained in:
parent
282bbc3709
commit
258c0998ab
1 changed files with 58 additions and 2 deletions
|
|
@ -2486,12 +2486,68 @@ async fn sync_agent_tmpfiles(agents: &[String]) -> Result<(String, String)> {
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::{
|
use super::{
|
||||||
PAUSED_MARKER_FILE, limits_dropin_body, redact_password_line, remove_marker_in,
|
OwnedFd, PAUSED_MARKER_FILE, PrivRequest, check_fd_agreement, limits_dropin_body,
|
||||||
write_state_file_nofollow,
|
redact_password_line, remove_marker_in, write_state_file_nofollow,
|
||||||
};
|
};
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
use std::sync::atomic::{AtomicU32, Ordering};
|
use std::sync::atomic::{AtomicU32, Ordering};
|
||||||
|
|
||||||
|
/// A request that streams into a caller-supplied descriptor.
|
||||||
|
fn fd_taking_request() -> PrivRequest {
|
||||||
|
PrivRequest::SendAgentSnapshotToFd {
|
||||||
|
agent_name: "atlas".to_owned(),
|
||||||
|
snapshot_name: "hive-migrate".to_owned(),
|
||||||
|
parent_snapshot_name: None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A real descriptor — `/dev/null` rather than a fake, so the drop
|
||||||
|
/// that closes it on a rejection path is genuinely exercised.
|
||||||
|
fn some_fd() -> OwnedFd {
|
||||||
|
std::fs::File::open("/dev/null")
|
||||||
|
.expect("open /dev/null")
|
||||||
|
.into()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An op that streams into a passed descriptor cannot invent one:
|
||||||
|
/// falling back to anything (a temp file, the response socket) would
|
||||||
|
/// send an agent's state somewhere the caller never asked for.
|
||||||
|
#[test]
|
||||||
|
fn an_fd_taking_op_without_a_descriptor_is_rejected() {
|
||||||
|
let err = check_fd_agreement(&fd_taking_request(), None)
|
||||||
|
.expect_err("no descriptor arrived, so this must not proceed");
|
||||||
|
let msg = format!("{err:#}");
|
||||||
|
assert!(msg.contains("requires a passed file descriptor"), "{msg}");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The mirror case: a descriptor sent alongside an op that takes
|
||||||
|
/// none is a protocol error, not something to ignore. Returning the
|
||||||
|
/// error drops the `OwnedFd`, which closes it — the alternative
|
||||||
|
/// leaks one descriptor per stray request in a long-lived root
|
||||||
|
/// process.
|
||||||
|
#[test]
|
||||||
|
fn a_descriptor_sent_to_an_op_that_takes_none_is_rejected() {
|
||||||
|
let fd = some_fd();
|
||||||
|
let err = check_fd_agreement(&PrivRequest::DaemonReload, Some(&fd))
|
||||||
|
.expect_err("an unexpected descriptor must not be silently ignored");
|
||||||
|
let msg = format!("{err:#}");
|
||||||
|
assert!(
|
||||||
|
msg.contains("does not take a passed file descriptor"),
|
||||||
|
"{msg}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Both agreeing combinations pass, so the check rejects mismatches
|
||||||
|
/// rather than descriptors in general.
|
||||||
|
#[test]
|
||||||
|
fn agreeing_combinations_are_accepted() {
|
||||||
|
let fd = some_fd();
|
||||||
|
check_fd_agreement(&fd_taking_request(), Some(&fd))
|
||||||
|
.expect("an fd-taking op with its descriptor is the normal case");
|
||||||
|
check_fd_agreement(&PrivRequest::DaemonReload, None)
|
||||||
|
.expect("every ordinary request arrives without a descriptor");
|
||||||
|
}
|
||||||
|
|
||||||
/// An unset weight is "not configured": the drop-in must come out
|
/// An unset weight is "not configured": the drop-in must come out
|
||||||
/// byte-identical to the pre-weights two-setting body, so a hive-c0re
|
/// byte-identical to the pre-weights two-setting body, so a hive-c0re
|
||||||
/// older than this field can't change what lands on disk.
|
/// older than this field can't change what lands on disk.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue