Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 60 additions & 8 deletions src/remote/attach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use super::{args::*, process::wait_with_output_timeout, restart_policy::*, shell
use base64::Engine as _;
use std::collections::BTreeMap;
use std::fs::{self, File};
use std::io::{self, IsTerminal, Write as _};
use std::io::{self, IsTerminal, Read as _, Write as _};
use std::path::{Path, PathBuf};
use std::process::{Command, Output, Stdio};
use std::process::{Child, Command, Output, Stdio};

use interprocess::local_socket::traits::Listener as _;
#[cfg(all(test, unix))]
Expand Down Expand Up @@ -594,6 +594,13 @@ impl RemoteSsh {
.stderr(Stdio::piped())
.spawn()?;

if !self.noninteractive {
return normalize_remote_output(output_with_forwarded_stderr(
child,
Some(script.as_bytes()),
)?);
}

let write_result = if let Some(mut stdin) = child.stdin.take() {
stdin.write_all(script.as_bytes())
} else {
Expand All @@ -602,11 +609,7 @@ impl RemoteSsh {
"ssh bootstrap stdin missing",
))
};
let output = if self.noninteractive {
wait_with_output_timeout(child, NONINTERACTIVE_SSH_COMMAND_TIMEOUT)?
} else {
child.wait_with_output()?
};
let output = wait_with_output_timeout(child, NONINTERACTIVE_SSH_COMMAND_TIMEOUT)?;
write_result?;
normalize_remote_output(output)
}
Expand All @@ -623,7 +626,7 @@ impl RemoteSsh {
let output = if self.noninteractive {
wait_with_output_timeout(command.spawn()?, NONINTERACTIVE_SSH_COMMAND_TIMEOUT)
} else {
command.output()
output_with_forwarded_stderr(command.spawn()?, None)
}?;
normalize_remote_output(output)
}
Expand Down Expand Up @@ -685,6 +688,55 @@ impl RemoteSsh {
}
}

// Only interactive setup uses this relay. Background probes retain their
// capture-only timeout path so SSH diagnostics cannot overwrite the active TUI.
fn output_with_forwarded_stderr(mut child: Child, stdin: Option<&[u8]>) -> io::Result<Output> {
let mut child_stderr = child
.stderr
.take()
.ok_or_else(|| io::Error::new(io::ErrorKind::BrokenPipe, "ssh command stderr missing"))?;
let stderr_relay = thread::spawn(move || -> io::Result<Vec<u8>> {
let mut captured = Vec::new();
let mut buffer = [0_u8; 8 * 1024];
let mut destination = io::stderr();

loop {
let read = child_stderr.read(&mut buffer)?;
if read == 0 {
break;
}
captured.extend_from_slice(&buffer[..read]);
if destination.write_all(&buffer[..read]).is_ok() {
let _ = destination.flush();
}
}

Ok(captured)
});

let write_result = if let Some(bytes) = stdin {
if let Some(mut child_stdin) = child.stdin.take() {
child_stdin.write_all(bytes)
} else {
Err(io::Error::new(
io::ErrorKind::BrokenPipe,
"ssh bootstrap stdin missing",
))
}
} else {
Ok(())
};
let output_result = child.wait_with_output();
let stderr_result = stderr_relay
.join()
.map_err(|_| io::Error::other("ssh stderr relay panicked"))?;

let mut output = output_result?;
write_result?;
output.stderr = stderr_result?;
Ok(output)
}

fn normalize_remote_output(mut output: Output) -> io::Result<Output> {
normalize_remote_stdout(&mut output.stdout, output.status.success())?;
Ok(output)
Expand Down
173 changes: 173 additions & 0 deletions tests/remote_attach.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
#![cfg(unix)]

use std::fs;
use std::io::{BufRead, BufReader};
use std::os::unix::fs::PermissionsExt;
use std::os::unix::process::CommandExt;
use std::path::{Path, PathBuf};
use std::process::{Child, Command, Stdio};
use std::sync::mpsc;
use std::thread;
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};

const CHECK_NOTICE: &str = "# Tailscale SSH requires an additional check.";
const CHECK_URL: &str = "# To authenticate, visit: https://login.tailscale.com/a/test";
const LATER_FAILURE: &str = "ssh: later setup probe failed";

struct TestCleanup {
temp_dir: PathBuf,
child: Option<Child>,
reader: Option<thread::JoinHandle<()>>,
}

impl Drop for TestCleanup {
fn drop(&mut self) {
if let Some(mut child) = self.child.take() {
// The child leads a private process group and has not been reaped.
// Kill its fake SSH descendants too, not just the Herdr launcher.
// SAFETY: the negative PID targets only this test's process group.
unsafe { libc::kill(-(child.id() as libc::pid_t), libc::SIGKILL) };
let _ = child.wait();
}
if let Some(reader) = self.reader.take() {
let _ = reader.join();
}
let _ = fs::remove_dir_all(&self.temp_dir);
}
}

fn wait_for_file(path: &Path, timeout: Duration) {
let deadline = Instant::now() + timeout;
while !path.exists() {
assert!(Instant::now() < deadline, "timed out waiting for fake ssh");
thread::sleep(Duration::from_millis(10));
}
}

#[test]
fn ssh_check_message_is_visible_while_authentication_waits() {
check_authentication_output(false);
check_authentication_output(true);
}

fn check_authentication_output(framed_shell: bool) {
let nonce = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("system clock after Unix epoch")
.as_nanos();
let temp_dir = std::env::temp_dir().join(format!(
"herdr-remote-auth-test-{}-{nonce}",
std::process::id()
));
let mut cleanup = TestCleanup {
temp_dir: temp_dir.clone(),
child: None,
reader: None,
};
fs::create_dir_all(&temp_dir).expect("create test directory");

let started_path = temp_dir.join("ssh-started");
let approval_path = temp_dir.join("ssh-approved");
let advanced_path = temp_dir.join("ssh-advanced");
let first_done_path = temp_dir.join("ssh-first-done");
let ssh_path = temp_dir.join("ssh");
fs::write(
&ssh_path,
format!(
r#"#!/bin/sh
authenticate() {{
: > "$FAKE_SSH_STARTED"
printf '%s\n%s\n' '{CHECK_NOTICE}' '{CHECK_URL}' >&2
while [ ! -e "$FAKE_SSH_APPROVED" ]; do
/bin/sleep 0.01
done
}}
if [ ! -e "$FAKE_SSH_FIRST_DONE" ]; then
: > "$FAKE_SSH_FIRST_DONE"
if [ "$FAKE_SSH_FRAMED" = 0 ]; then authenticate; fi
/bin/cat >/dev/null
printf 'login banner\nherdr-remote-output-ready:1\nLinux\nx86_64\n'
exit 0
fi
if [ "$FAKE_SSH_FRAMED" = 1 ] && [ ! -e "$FAKE_SSH_STARTED" ]; then
authenticate
fi
/bin/cat >/dev/null
: > "$FAKE_SSH_ADVANCED"
printf '%s\n' '{LATER_FAILURE}' >&2
exit 255
"#
),
)
.expect("write fake ssh");
fs::set_permissions(&ssh_path, fs::Permissions::from_mode(0o755))
.expect("make fake ssh executable");

let inherited_path = std::env::var("PATH").unwrap_or_default();
let path = format!("{}:{inherited_path}", temp_dir.display());
let child = Command::new(env!("CARGO_BIN_EXE_herdr"))
.args(["--remote", "check-host"])
.env("PATH", path)
.env("FAKE_SSH_FRAMED", if framed_shell { "1" } else { "0" })
.env("FAKE_SSH_STARTED", &started_path)
.env("FAKE_SSH_APPROVED", &approval_path)
.env("FAKE_SSH_ADVANCED", &advanced_path)
.env("FAKE_SSH_FIRST_DONE", &first_done_path)
.env("HERDR_CONFIG_PATH", temp_dir.join("config.toml"))
.env_remove("HERDR_ENV")
.env_remove("HERDR_SESSION")
.env_remove("HERDR_SOCKET_PATH")
.env_remove("HERDR_CLIENT_SOCKET_PATH")
.env_remove("HERDR_REMOTE_BINARY")
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::piped())
.process_group(0)
.spawn()
.expect("start remote attach");

cleanup.child = Some(child);
let child = cleanup.child.as_mut().expect("registered child");
let stderr = child.stderr.take().expect("remote attach stderr");
let (line_tx, line_rx) = mpsc::channel();
cleanup.reader = Some(thread::spawn(move || {
for line in BufReader::new(stderr).lines() {
if line_tx
.send(line.expect("read remote attach stderr"))
.is_err()
{
break;
}
}
}));

wait_for_file(&started_path, Duration::from_secs(2));
let notice = line_rx.recv_timeout(Duration::from_secs(2));
let url = line_rx.recv_timeout(Duration::from_secs(2));
fs::write(&approval_path, b"approved").expect("release fake ssh approval");
wait_for_file(&advanced_path, Duration::from_secs(2));

let status = child.wait().expect("wait for remote attach");
cleanup.child = None;
cleanup
.reader
.take()
.expect("registered stderr reader")
.join()
.expect("join stderr reader");
let later_lines = line_rx.try_iter().collect::<Vec<_>>();

assert_eq!(notice.as_deref(), Ok(CHECK_NOTICE));
assert_eq!(url.as_deref(), Ok(CHECK_URL));
assert!(
later_lines.iter().any(|line| line == LATER_FAILURE),
"later SSH stderr should also be visible: {later_lines:?}"
);
assert!(
later_lines.iter().any(|line| {
line.contains("error: remote binary discovery failed") && line.contains(LATER_FAILURE)
}),
"failed SSH stderr should remain in the contextual error: {later_lines:?}"
);
assert!(!status.success());
}
Loading