feat: add MethodCall::push_fd() for passing file descriptors (SCM_RIGHTS)

[lsjostro]

systemd's sd-varlink extends varlink with file-descriptor passing over the
Unix socket via SCM_RIGHTS: fds are pushed before the call and referenced from
the parameters by their push-order index. (This is not part of the core varlink
protocol as specified on varlink.org — the push-order convention is the de-facto
one used by systemd's services.) This crate had no client-side way to attach
fds; this adds that, mirroring sd_varlink_push_fd().

API

let mut call = client.some_method(args);
let idx = call.push_fd(fd)?;   // -> 0, then 1, ... (push order == the index methods reference)
call.more()?;                  // or .call()/.oneway(); fds are delivered with the request

• MethodCall::push_fd(fd) -> io::Result<usize> (unix only). Dups the fd with
  F_DUPFD_CLOEXEC (caller keeps ownership of the original), queues it, and
  returns its push-order index.
• On send, queued fds are delivered with the request via a single
  sendmsg() + SCM_RIGHTS on the connection's socket; any bytes past the first
  sendmsg are written normally through the existing writer.
• Only AF_UNIX-socket-backed connections support this — SCM_RIGHTS does not
  exist elsewhere. push_fd checks the socket family up front (getsockname),
  so a tcp: connection or a reader/writer-pair connection fails immediately
  with io::ErrorKind::Unsupported rather than later at send time.

No opt-in gate (cf. systemd's SD_VARLINK_SERVER_ALLOW_FD_PASSING_OUTPUT):
on the client side, calling push_fd is itself the explicit opt-in — no
ancillary data is ever attached unless the application pushed an fd for that
specific call.

Scope

Send-only. Receiving descriptors (server → client) is not implemented — it would
require recvmsg-based reading throughout the input path, and nothing needs it yet.
cfg(unix) only, matching the crate's existing platform split; libc is already
a unix dependency.

Tests

varlink/src/test.rs:

• test_push_fd_passes_descriptor: push an fd over one end of a socketpair,
  recvmsg() the other end, and confirm the descriptor arrives and refers to the
  same underlying pipe (write through the received fd, read it back).
• test_push_fd_multiple_and_cleared_after_send: two fds arrive in one
  SCM_RIGHTS control message with consecutive indices and push order preserved;
  the queue is cleared on send, so a second send carries no ancillary data.
• test_push_fd_requires_socket: push_fd on a non-socket connection returns
  Unsupported.
• test_push_fd_requires_unix_socket: push_fd on a TCP-backed connection
  returns Unsupported.

cargo test -p varlink passes.

[haraldh]

Review: MethodCall::push_fd()

Reviewed with local verification — built and ran the tests on macOS, and cross-checked the fd-passing claim against the varlink spec and systemd's sd-varlink.

🔴 Critical — will fail CI on Windows

unused_mut on sent (varlink/src/lib.rs, in send())

let mut sent = false;
#[cfg(unix)]
if !self.fds.is_empty() { … sent = true; }
if !sent { w.write_all(&b)…?; }

On non-unix builds the only mutation (sent = true) is inside the #[cfg(unix)] block, so sent is never mutated → variable does not need to be mutable. lint.yml runs clippy --all-features -- -D warnings on windows-latest, which promotes that warning to an error. Confirmed by reproduction with rustc -W unused.

Suggested fix — resolve the flag via cfg instead of a mutable binding:

#[cfg(unix)]
let sent = if !self.fds.is_empty() {
    let sock = conn.stream.as_ref().map(|s| s.as_raw_fd())
        .ok_or_else(|| MError::from(context!(ErrorKind::ConnectionBusy)))?;
    let n = sendmsg_with_fds(sock, &b, &self.fds).map_err(map_context!())?;
    self.fds.clear();
    if n < b.len() { w.write_all(&b[n..]).map_err(map_context!())?; }
    true
} else { false };
#[cfg(not(unix))]
let sent = false;
if !sent { w.write_all(&b).map_err(map_context!())?; }

🟠 Important — protocol accuracy

The PR description says "Varlink lets a method call carry file descriptors as ancillary data (SCM_RIGHTS), referenced from the parameters by push order." That overstates it:

• fd passing is not part of the varlink protocol as specified on varlink.org. Neither Interface-Definition nor Method-Call mentions file descriptors, SCM_RIGHTS, or ancillary data. The wire format there is just NUL-terminated JSON over a stream.
• It is a systemd sd-varlink extension. This PR's push_fd mirrors sd_varlink_push_fd(), including returning the push-order index — confirmed in sd-varlink.c:

  int i = (int) v->n_pushed_fds;
  v->pushed_fds[v->n_pushed_fds++] = fd;
  return i;

• The "referenced from the parameters by push order" convention is de-facto (a param field carries the integer index); it isn't written down even in the sd_varlink_push_fd(3) man page, which documents the return only as "a non-negative integer."

Suggested description: "systemd's sd-varlink extends varlink with file-descriptor passing over the Unix socket via SCM_RIGHTS; fds are pushed before the call and referenced from the parameters by their push-order index. This mirrors sd_varlink_push_fd()."

Related design gap: systemd gates fd passing behind an explicit opt-in on both ends (SD_VARLINK_SERVER_ALLOW_FD_PASSING_OUTPUT / JSON_STREAM_ALLOW_FD_PASSING_OUTPUT); sd_varlink_push_fd returns -EPERM when it isn't enabled. This PR always attaches SCM_RIGHTS whenever push_fd was called, with no negotiation — a peer not expecting ancillary data silently receives extra fds. Worth deciding whether to mirror the opt-in gating.

🟠 Important — misleading doc / late failure on TCP

push_fd gates only on stream.is_none(), and the doc says it works on any connection created via Connection::with_address. But SCM_RIGHTS is AF_UNIX-only. A tcp: connection has stream = Some(...), so push_fd succeeds and the failure is deferred to call() where sendmsg fails with EINVAL. It's a loud failure (error returned, nothing silent), but the doc should say "Unix-socket-backed", and ideally the check would reject non-Unix sockets at push_fd time.

🟡 Suggestions

• Test coverage: the two tests are solid — test_push_fd_passes_descriptor does a real recvmsg and proves the fd works through a pipe. Missing behavioral cases worth adding: multiple push_fd calls (verify returned indices 0,1,… and that all arrive in one control message), and that fds is cleared after a send so a second send doesn't re-attach.

🟢 Strengths

• Correct write ordering: w (boxed writer) and conn.stream are both unbuffered try_clone()s of the same socket (stream.rs), so sendmsg + tail write_all preserve byte order — no hidden BufWriter.
• Sound unsafe/libc usage: CMSG_SPACE/CMSG_LEN/CMSG_FIRSTHDR used correctly; size_of_val sizes N fds into one control message; as _ casts handle glibc/musl msghdr width differences (documented).
• Good ownership model: F_DUPFD_CLOEXEC (min fd 3, avoids std streams), fds held as OwnedFd and cleared post-send. Comments accurately describe this.
• Clean #[cfg(unix)] gating; clippy clean on unix, and builds + both tests pass on macOS (MSG_NOSIGNAL is defined and honored there).

Note

cargo test --all-features currently fails to compile due to a pre-existing tokio::process feature-gating issue in client_async.rs, unrelated to this PR (present on master).

[lsjostro]

Thanks for the thorough review, @haraldh — all points addressed in 0aa089b:

• unused_mut on non-unix: took your suggested fix — sent is now resolved via #[cfg(unix)] / #[cfg(not(unix))] bindings instead of a mutable flag.
• Protocol accuracy: you're right, this is systemd's sd-varlink extension, not core varlink. Reworded the PR description with your suggested text and updated the push_fd rustdoc to say it mirrors sd_varlink_push_fd(3) and is not part of the varlink.org spec.
• TCP late failure: push_fd now checks the socket family via getsockname and rejects non-AF_UNIX sockets immediately with ErrorKind::Unsupported; docs say "AF_UNIX-socket-backed". Added test_push_fd_requires_unix_socket covering the tcp:-style case.
• Tests: added test_push_fd_multiple_and_cleared_after_send — two fds pushed (indices 0, 1), both arrive in a single SCM_RIGHTS control message with push order preserved (verified by writing distinct payloads through each received fd), and a second send on the same connection carries no ancillary data (queue cleared).

On the opt-in gating: I'd argue the client side doesn't need one — calling push_fd is itself the explicit per-call opt-in, so a peer never receives ancillary data unless the application deliberately attached fds to that specific call. systemd's SD_VARLINK_SERVER_ALLOW_FD_PASSING_* flags guard a generic server/connection object where fds can show up unrequested; the closest client-side analogue would matter once fd receiving lands. Happy to add a Connection-level enable flag if you'd prefer to mirror sd-varlink exactly.

cargo test -p varlink, cargo clippy -p varlink --all-targets -- -D warnings, and cargo fmt --check are clean on unix.

[coveralls]

Coverage Report for CI Build 30100250885

Warning

No base build found for commit 4f73812 on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 56.305%

Details

• Patch coverage: 4 uncovered changes across 1 file (222 of 226 lines covered, 98.23%).

Uncovered Changes

File	Changed	Covered	%
varlink/src/lib.rs	70	66	94.29%
Total (2 files)	226	222	98.23%

Coverage Regressions

Requires a base build to compare against. How to fix this →

---

Coverage Stats

Relevant Lines:	5408
Covered Lines:	3045
Line Coverage:	56.31%
Coverage Strength:	13.6 hits per line

---

💛 - Coveralls

[haraldh]

Thank you!