diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index 2e6f149b..71243614 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -3251,6 +3251,7 @@ mod tests { tty7_core::daemon::protocol::PaneProcs { procs: vec![proc_entry(100, "zsh", 0, true)], ports: Vec::new(), + probe: Default::default(), } } @@ -3262,6 +3263,7 @@ mod tests { proc_entry(101, "cargo", 1, true), ], ports: Vec::new(), + probe: Default::default(), } } diff --git a/crates/tty7-cli/src/output.rs b/crates/tty7-cli/src/output.rs index f6381c6e..5d3294c2 100644 --- a/crates/tty7-cli/src/output.rs +++ b/crates/tty7-cli/src/output.rs @@ -4,7 +4,7 @@ use tty7_core::core::machine::{Machine, PaneNode, Workspace}; use tty7_core::core::session::WorkspaceId; use tty7_core::core::tab_view::{TabLabel, TabView, strip_host_prefix, tab_views_of}; use tty7_core::daemon::control::{PaneAgentState, RouteInfo, ServerStatus}; -use tty7_core::daemon::protocol::{PaneInfo, PaneProcs}; +use tty7_core::daemon::protocol::{PaneInfo, PaneProcs, PortProbe}; use crate::resolve; @@ -236,9 +236,34 @@ fn render_node(out: &mut String, node: &PaneNode, machine: &Machine, depth: usiz } } +/// What the port probe has to say for itself, when it has something to say. +/// +/// This is the line that makes `tty7 procs` a diagnostic rather than another +/// place to read an empty PORTS table. An empty table means "nothing is +/// listening" only when the probe actually ran, and until it said so there was +/// no way — from a screenshot, from a bug report, from the JSON — to tell that +/// case from a probe that never got off the ground. +fn probe_note(probe: &PortProbe) -> Option { + match probe { + PortProbe::Ok => None, + PortProbe::Restricted => Some( + "note: some processes here belong to another user; a probe running as you \ + cannot see their sockets" + .to_string(), + ), + PortProbe::Unavailable(detail) => Some(format!( + "note: could not check for listening ports ({detail})" + )), + } +} + pub fn procs_tables(procs: &PaneProcs) -> String { + let note = probe_note(&procs.probe); if procs.procs.is_empty() && procs.ports.is_empty() { - return "nothing running in this pane\n".to_string(); + return match note { + Some(note) => format!("nothing running in this pane\n{note}\n"), + None => "nothing running in this pane\n".to_string(), + }; } let rows: Vec> = procs .procs @@ -261,6 +286,11 @@ pub fn procs_tables(procs: &PaneProcs) -> String { .collect(); out.push_str(&table(&["PORT", "PID", "NAME"], &rows)); } + if let Some(note) = note { + out.push('\n'); + out.push_str(¬e); + out.push('\n'); + } out } @@ -551,6 +581,7 @@ mod tests { addr: "*".into(), name: "node".into(), }], + probe: PortProbe::Ok, }; let rendered = procs_tables(&procs); assert!( @@ -562,5 +593,54 @@ mod tests { "the foreground process is marked: {rendered}" ); assert!(rendered.contains("3000"), "{rendered}"); + assert!( + !rendered.contains("note:"), + "a probe that worked says nothing: {rendered}" + ); + } + + /// #731's diagnostic. Someone whose ports are missing runs `tty7 procs` + /// and pastes what it says; an empty PORTS table is only worth pasting if + /// it distinguishes a quiet pane from a probe that never ran. + #[test] + fn a_probe_that_could_not_run_says_so_next_to_the_empty_table() { + let mut procs = PaneProcs { + procs: vec![ProcEntry { + pid: 100, + name: "zsh".into(), + depth: 0, + foreground: true, + }], + ports: Vec::new(), + probe: PortProbe::Unavailable("lsof: program not found".into()), + }; + let rendered = procs_tables(&procs); + assert!( + rendered.contains("could not check for listening ports"), + "{rendered}" + ); + assert!( + rendered.contains("lsof: program not found"), + "the reason is the whole point of the line: {rendered}" + ); + + procs.probe = PortProbe::Restricted; + assert!( + procs_tables(&procs).contains("another user"), + "{}", + procs_tables(&procs) + ); + + // And on a pane with nothing running at all, where the tables are not + // drawn, the note still has to come out. + let empty = PaneProcs { + probe: PortProbe::Unavailable("lsof: program not found".into()), + ..Default::default() + }; + assert!( + procs_tables(&empty).contains("could not check"), + "{}", + procs_tables(&empty) + ); } } diff --git a/crates/tty7-core/src/daemon/procinfo.rs b/crates/tty7-core/src/daemon/procinfo.rs index 30a8fe85..04897d8e 100644 --- a/crates/tty7-core/src/daemon/procinfo.rs +++ b/crates/tty7-core/src/daemon/procinfo.rs @@ -1,21 +1,187 @@ use std::collections::HashMap; -use crate::daemon::protocol::{PaneProcs, PortEntry, ProcEntry}; +use crate::daemon::protocol::{PaneProcs, PortEntry, PortProbe, ProcEntry}; const MAX_DEPTH: u8 = 6; +/// How many processes the panel is asked to draw. const MAX_PROCS: usize = 64; +/// How far the walk itself goes before it calls the tree pathological. +/// +/// This is deliberately far above `MAX_PROCS`, and the gap is the point. The +/// walk is depth-first over children sorted by ascending pid, so capping it at +/// the number of rows the panel wants meant one busy early branch — a build, +/// a container runtime, an agent's worker pool — could consume the whole +/// budget before the traversal ever reached the pane's newest child. A server +/// someone just started is the *last* pid in that ordering, so the one process +/// the Ports section exists for was the one most likely to fall off the end, +/// and it fell off silently. The port probe is asked about everything the walk +/// found; only the list handed to the panel is cut back to `MAX_PROCS`. +const MAX_TREE: usize = 512; + pub fn snapshot(shell_pid: u32, fg_pgid: Option) -> PaneProcs { let table = process_table(); let procs = walk(&table, shell_pid, fg_pgid); - let ports = listening_ports(&procs); - PaneProcs { procs, ports } + let (ports, probe) = listening_ports(&procs); + let probe = match probe.is_ok() && tree_has_foreign_uid(&table, &procs, current_uid()) { + true => PortProbe::Restricted, + false => probe, + }; + if let PortProbe::Unavailable(detail) = &probe { + note_probe_failure(shell_pid, detail); + } + finish(procs, ports, probe) +} + +/// How long the same probe failure waits before it is written down again. +/// +/// `snapshot` answers one `QueryProcs`, and the Info panel sends one every two +/// seconds for as long as it is open — a probe that cannot run now will not +/// have started working two seconds later. Logging every attempt turns one +/// standing fact into thirty lines a minute in a file that truncates itself at +/// 4 MiB, which costs the reporter the rest of the session they turned logging +/// on to capture. Once a minute still leaves a trail for a failure that +/// outlives the panel. +const PROBE_LOG_GAP: std::time::Duration = std::time::Duration::from_secs(60); + +fn note_probe_failure(shell_pid: u32, detail: &str) { + static LAST: std::sync::Mutex> = + std::sync::Mutex::new(None); + let line = format!("listening-port probe failed for pane shell {shell_pid}: {detail}"); + let Ok(mut last) = LAST.lock() else { return }; + if probe_log_due(&mut last, &line, std::time::Instant::now(), PROBE_LOG_GAP) { + log::warn!("{line}"); + } +} + +/// Whether `line` is worth a log entry now, given what was written last. +/// +/// A line that has not been said before is always worth saying — a probe that +/// starts failing for a second reason, or a second pane failing for the same +/// one, is news. Repeating one is worth it only once per `gap`. +fn probe_log_due( + last: &mut Option<(String, std::time::Instant)>, + line: &str, + now: std::time::Instant, + gap: std::time::Duration, +) -> bool { + if let Some((said, at)) = last.as_ref() { + if said == line && now.duration_since(*at) < gap { + return false; + } + } + *last = Some((line.to_string(), now)); + true +} + +/// The answer as the panel gets it: the process list trimmed to what a sidebar +/// can show, and the ports left whole. +/// +/// Trimming here rather than in `walk` is what keeps a port owned by the 100th +/// process in the tree on screen — the row names its owner out of the full +/// list, so cutting the list afterwards costs the panel a process row it had +/// no room for and costs the Ports section nothing. +fn finish(mut procs: Vec, ports: Vec, probe: PortProbe) -> PaneProcs { + procs.truncate(MAX_PROCS); + PaneProcs { + procs, + ports, + probe, + } +} + +/// Whether any process in the pane's tree belongs to a user other than `me`. +/// +/// `sudo go run main.go` is the shape this is about. The process tree still +/// walks — the kernel will name another user's processes — but the sockets +/// they hold are readable only by their owner or by root, so `lsof` running as +/// this user answers "nothing is listening" about a server that plainly is. +/// Saying "some of these are another user's" is the difference between a panel +/// that is wrong and a panel that is honest. +fn tree_has_foreign_uid(table: &HashMap, procs: &[ProcEntry], me: u32) -> bool { + // Root sees everyone's sockets, so nothing is hidden from a daemon that is + // already root and there is nothing to warn about. + if me == 0 { + return false; + } + procs.iter().any(|p| { + table + .get(&p.pid) + .is_some_and(|row| row.uid != me && confirm_foreign(p.pid, me)) + }) +} + +/// A second opinion on a row that looks like another user's. +/// +/// Asked only about the pane's own tree, and only about the rows that already +/// look foreign, so an ordinary pane pays nothing for it and a `sudo` pays one +/// file read. +/// +/// Linux needs it because the owner of `/proc/` is not always the process's +/// uid: the kernel makes that directory `root:root` whenever a process's +/// dumpable attribute has been cleared, which is what executing a set-user-ID +/// binary or one carrying file capabilities does. A plain `ping` in a pane is +/// the user's own process behind a root-owned `/proc` entry, and taking the +/// directory's word for it would have the panel apologise for sockets it can +/// read perfectly well — the opposite mistake to the one this file is fixing, +/// and just as wrong. `Uid:` in `/proc//status` is the real answer and is +/// readable either way. +#[cfg(target_os = "linux")] +fn confirm_foreign(pid: u32, me: u32) -> bool { + match std::fs::read_to_string(format!("/proc/{pid}/status")) { + Ok(text) => status_euid(&text).is_none_or(|uid| uid != me), + // Gone, or unreadable: keep the directory's verdict rather than + // inventing a second one out of a failed read. + Err(_) => true, + } +} + +/// macOS reads the effective uid straight out of `PROC_PIDTBSDINFO`, and +/// Windows has no uid to be wrong about, so there is nothing to confirm. +#[cfg(not(target_os = "linux"))] +fn confirm_foreign(_pid: u32, _me: u32) -> bool { + true +} + +/// The effective uid on the `Uid:` line of a `/proc//status`, which reads +/// `Uid:\t\t\t\t`. +/// +/// The effective one is the second: it is the credential the kernel checks when +/// something asks to read the process's sockets. +#[cfg_attr(not(target_os = "linux"), allow(dead_code))] +fn status_euid(text: &str) -> Option { + text.lines() + .find_map(|line| line.strip_prefix("Uid:"))? + .split_whitespace() + .nth(1)? + .parse() + .ok() +} + +#[cfg(unix)] +fn current_uid() -> u32 { + // SAFETY: `getuid` reads the calling process's own credentials and cannot + // fail. + unsafe { libc::getuid() } +} + +/// Windows has no uid, and its port probe is a kernel table rather than a +/// subprocess with an identity — `Row::uid` is 0 there and so is this, so the +/// check above is a constant false. +#[cfg(not(unix))] +fn current_uid() -> u32 { + 0 } struct Row { ppid: u32, pgid: u32, + /// The effective uid of the process, which is what decides whether this + /// daemon may look at its sockets. 0 on platforms that have no such thing, + /// and on Linux the cheapest reading of it rather than the last word — see + /// `confirm_foreign`. + uid: u32, name: String, } @@ -32,7 +198,7 @@ fn walk(table: &HashMap, shell_pid: u32, fg_pgid: Option) -> Vec< let mut stack = vec![(shell_pid, 0u8)]; while let Some((pid, depth)) = stack.pop() { let Some(row) = table.get(&pid) else { continue }; - if out.len() >= MAX_PROCS { + if out.len() >= MAX_TREE { break; } out.push(ProcEntry { @@ -96,6 +262,10 @@ fn process_table() -> HashMap { Row { ppid: info.pbi_ppid, pgid: info.pbi_pgid, + // The effective uid, not the real one: a setuid `sudo` still + // runs as the user who typed it, and it is the effective uid + // that decides whose sockets `lsof` may read. + uid: info.pbi_uid, name, }, ); @@ -146,7 +316,26 @@ fn process_table() -> HashMap { .rfind('(') .map_or_else(|| String::new(), |open| stat[open + 1..close].to_string()) }); - table.insert(pid, Row { ppid, pgid, name }); + // `/proc/` is normally owned by the process's effective uid, which + // is the one that governs who may read its sockets. Normally: the + // kernel hands the directory to `root` for a process whose dumpable + // attribute it cleared, so this is a cheap first pass over every pid on + // the machine and `confirm_foreign` settles the few that look foreign + // and are in a pane's tree. A stat that fails on a pid whose `stat` + // file just parsed is a race with the process exiting; calling that + // "mine" keeps a dying process from being mistaken for another user's. + let uid = std::fs::metadata(format!("/proc/{pid}")) + .map(|m| std::os::unix::fs::MetadataExt::uid(&m)) + .unwrap_or_else(|_| current_uid()); + table.insert( + pid, + Row { + ppid, + pgid, + uid, + name, + }, + ); } table } @@ -161,6 +350,7 @@ fn process_table() -> HashMap { Row { ppid: p.parent, pgid: 0, + uid: 0, name: p.name, }, ) @@ -221,20 +411,42 @@ pub(super) fn proc_name(pid: i32) -> Option { (!comm.is_empty()).then(|| comm.to_string()) } +/// Where to look for `lsof`, in order. +/// +/// `PATH` first, and then the two absolute paths it actually lives at, because +/// the daemon's `PATH` is not the shell's. macOS ships `lsof` in `/usr/sbin`, +/// which is on the default login `PATH` and is exactly the kind of entry a +/// hand-written `export PATH=...` in a dotfile drops on the floor — and the +/// daemon inherits whatever the app that launched it had. Falling back to the +/// absolute path costs one failed `execvp` in the case that used to end with +/// the panel quietly claiming nothing was listening. #[cfg(unix)] -fn listening_ports(procs: &[ProcEntry]) -> Vec { - use std::process::{Command, Stdio}; +const LSOF_CANDIDATES: [&str; 3] = ["lsof", "/usr/sbin/lsof", "/usr/bin/lsof"]; +/// How long the probe gets before it is declared hung. +/// +/// `lsof` is famous for blocking on a wedged network mount, and this one runs +/// on the daemon's connection thread: without a bound, one stuck call does not +/// merely lose a port, it stops the pane answering `QueryProcs` at all, for as +/// long as the mount stays wedged. The Info panel re-polls every two seconds, +/// so a probe still running after three has already missed its slot. +#[cfg(unix)] +const PROBE_BUDGET: std::time::Duration = std::time::Duration::from_secs(3); + +#[cfg(unix)] +fn listening_ports(procs: &[ProcEntry]) -> (Vec, PortProbe) { if procs.is_empty() { - return Vec::new(); + return (Vec::new(), PortProbe::Ok); } let pid_list = procs .iter() .map(|p| p.pid.to_string()) .collect::>() .join(","); - let out = Command::new("lsof") - .args([ + let mut first_err = String::new(); + for tool in LSOF_CANDIDATES { + let mut cmd = std::process::Command::new(tool); + cmd.args([ "-nP", "-iTCP", "-sTCP:LISTEN", @@ -242,13 +454,113 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { "-p", &pid_list, "-Fpn", - ]) - .stdin(Stdio::null()) - .stderr(Stdio::null()) - .output(); - let Ok(out) = out else { return Vec::new() }; - let text = String::from_utf8_lossy(&out.stdout); + ]); + match run_bounded(cmd, PROBE_BUDGET) { + Ok(Run::Finished(out)) => { + let ports = parse_lsof(&String::from_utf8_lossy(&out.stdout), procs); + // The exit status is deliberately not read as failure. `lsof` + // returns 1 for a pid it could not locate, and a pane's tree + // grows and loses processes between the walk and this call as + // a matter of course — treating that as a broken probe would + // put a doubt on screen every time a `ls` finished. What the + // status is worth is a log line when the run both complained + // and came back with nothing. + if ports.is_empty() && !out.status.success() { + if let Some(line) = String::from_utf8_lossy(&out.stderr) + .lines() + .find(|l| !l.trim().is_empty()) + { + log::debug!("{tool} found no listeners and said: {line}"); + } + } + return (ports, PortProbe::Ok); + } + Ok(Run::TimedOut) => { + return ( + Vec::new(), + PortProbe::Unavailable(format!( + "{tool} did not answer within {}s", + PROBE_BUDGET.as_secs() + )), + ); + } + // Try the next candidate: this one is not there, or is not + // runnable. The complaint kept is the first one, about the name as + // the daemon's `PATH` sees it, since that is the failure worth + // reading — the others are fallbacks nobody asked for. + Err(e) => { + if first_err.is_empty() { + first_err = format!("{tool}: {e}"); + } + } + } + } + ( + Vec::new(), + PortProbe::Unavailable(format!( + "{first_err} (also tried {})", + LSOF_CANDIDATES[1..].join(", ") + )), + ) +} +/// What became of a probe process. +/// +/// Compiled everywhere and used by the unix probe and by the tests, which is +/// how a parser and a timeout that only ever run on macOS and Linux get +/// exercised on a Windows machine. +#[cfg_attr(not(unix), allow(dead_code))] +enum Run { + Finished(std::process::Output), + TimedOut, +} + +/// Run `cmd` to completion, or kill it once `budget` is up. +/// +/// `Command::output` has no deadline, and the caller is a daemon thread that a +/// hung child would own forever. Both pipes are read after the wait rather +/// than while it runs, which is safe for a probe whose whole output is a few +/// hundred bytes and which is killed if it ever stops making progress. +/// +/// A killed run is reaped and its output abandoned unread. Reading it would +/// reintroduce the hang this exists to prevent: a child that spawned something +/// of its own hands the write end of the pipe on, and waiting for end-of-file +/// then means waiting for a grandchild nobody killed. +#[cfg_attr(not(unix), allow(dead_code))] +fn run_bounded( + mut cmd: std::process::Command, + budget: std::time::Duration, +) -> std::io::Result { + use std::process::Stdio; + + let mut child = cmd + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + let deadline = std::time::Instant::now() + budget; + loop { + if child.try_wait()?.is_some() { + return Ok(Run::Finished(child.wait_with_output()?)); + } + if std::time::Instant::now() >= deadline { + let _ = child.kill(); + // Reaps the process this call started, so a timed-out probe leaves + // no zombie behind; the pipes close as `child` drops. + let _ = child.wait(); + return Ok(Run::TimedOut); + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } +} + +/// The listeners in one `lsof -Fpn` report, named after the processes that own +/// them. +/// +/// Split out from the call so the format can be tested off a Mac: this parser +/// is the half of the probe that has no platform in it. +#[cfg_attr(not(unix), allow(dead_code))] +fn parse_lsof(text: &str, procs: &[ProcEntry]) -> Vec { let by_pid: HashMap = procs.iter().map(|p| (p.pid, p.name.as_str())).collect(); let mut ports: Vec = Vec::new(); let mut current = 0u32; @@ -262,32 +574,13 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { let Some((addr, port)) = parse_listen_addr(rest) else { continue; }; - // One process listening on the same port over IPv4 and IPv6 is - // one port to show. Which of the two lines survives used to be - // whichever lsof printed first; now that the address is carried - // through to a clickable URL, the reachable one wins — a - // process bound to both `192.168.1.5` and `*` is on localhost, - // and the row should say so. - if let Some(seen) = ports - .iter_mut() - .find(|e| e.port == port && e.pid == current) - { - if !PortEntry::reaches_loopback(&seen.addr) && PortEntry::reaches_loopback(addr) - { - seen.addr = addr.to_string(); - } - continue; - } - ports.push(PortEntry { + record_listener( + &mut ports, + by_pid.get(¤t).copied().unwrap_or_default(), port, - pid: current, - addr: addr.to_string(), - name: by_pid - .get(¤t) - .copied() - .unwrap_or_default() - .to_string(), - }); + current, + addr.to_string(), + ); } _ => {} } @@ -311,8 +604,15 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { /// second call when its table outgrew that. The Info tab re-polls every two /// seconds while it is open, so this is a fixed handful of microseconds, with /// no process spawn and nothing allocated per pid. +/// +/// There is no tool to be missing here and no subprocess to hang, so the state +/// is `Ok` whenever the kernel answered at all — including for a machine with +/// IPv6 off, which is answered for by its IPv4 table alone. The one case left +/// is a `GetExtendedTcpTable` that would not answer for either family, and that +/// is `Unavailable` for the same reason a missing `lsof` is: nothing looked, so +/// the empty list is not an answer. #[cfg(windows)] -fn listening_ports(procs: &[ProcEntry]) -> Vec { +fn listening_ports(procs: &[ProcEntry]) -> (Vec, PortProbe) { use std::net::{Ipv4Addr, Ipv6Addr}; use windows_sys::Win32::NetworkManagement::IpHelper::{ @@ -321,18 +621,19 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { }; if procs.is_empty() { - return Vec::new(); + return (Vec::new(), PortProbe::Ok); } let by_pid: HashMap = procs.iter().map(|p| (p.pid, p.name.as_str())).collect(); let mut ports: Vec = Vec::new(); let v4 = tcp_table(AF_INET); - // SAFETY: `tcp_table` hands back either an empty buffer or one the kernel - // filled with a `MIB_TCPTABLE_OWNER_PID`; the `Vec` gives it the 4-byte - // alignment every field of that struct wants, and `rows` is clamped to what - // the buffer can actually hold before anything is read out of it. + // SAFETY: `tcp_table` hands back a buffer the kernel filled with a + // `MIB_TCPTABLE_OWNER_PID`, or nothing at all; the `Vec` gives it the + // 4-byte alignment every field of that struct wants, and `rows` is clamped + // to what the buffer can actually hold before anything is read out of it. unsafe { - if let Some((rows, count)) = table_rows::(&v4) + if let Some((rows, count)) = + table_rows::(v4.as_deref().unwrap_or(&[])) { for i in 0..count { let row = &*rows.add(i); @@ -357,9 +658,9 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { let v6 = tcp_table(AF_INET6); // SAFETY: as above, for the IPv6 shape of the same table. unsafe { - if let Some((rows, count)) = - table_rows::(&v6) - { + if let Some((rows, count)) = table_rows::( + v6.as_deref().unwrap_or(&[]), + ) { for i in 0..count { let row = &*rows.add(i); let Some(name) = by_pid.get(&row.dwOwningPid) else { @@ -378,7 +679,25 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { } ports.sort_by_key(|e| (e.port, e.pid)); - ports + (ports, windows_probe(v4.is_some(), v6.is_some())) +} + +/// What a Windows probe is worth, given whether each family's table could be +/// read. +/// +/// One family refusing is not a broken probe: a machine with IPv6 switched off +/// is an ordinary machine, and its IPv4 listeners are the whole truth about it. +/// Both refusing is the failure this file is about — nothing was looked at, and +/// an empty list then means "we do not know", which is the one thing #731 says +/// the panel must not spell as "None". +#[cfg(windows)] +fn windows_probe(v4: bool, v6: bool) -> PortProbe { + match v4 || v6 { + true => PortProbe::Ok, + false => PortProbe::Unavailable( + "GetExtendedTcpTable would not answer for either address family".to_string(), + ), + } } /// The two Winsock address families, named here rather than by switching on @@ -390,13 +709,17 @@ const AF_INET: u32 = 2; const AF_INET6: u32 = 23; /// One `GetExtendedTcpTable` snapshot of the listening sockets in `family`, as -/// the raw buffer the kernel filled, or an empty buffer if it would not answer. +/// the raw buffer the kernel filled, or `None` if it would not answer. /// -/// Failure is soft, the way an absent `lsof` is soft on unix: the panel shows no -/// ports rather than an error. A machine with IPv6 disabled takes that path for -/// `AF_INET6` alone and still gets its IPv4 ports. +/// A machine with IPv6 disabled takes the `None` path for `AF_INET6` alone and +/// still gets its IPv4 ports. What `None` must not do is disappear: it used to +/// come back as an empty buffer that read exactly like a family with no +/// listeners, so a table the kernel refused twice — or refused outright, which +/// a filter driver sitting on `iphlpapi` is enough to cause — left the panel +/// saying "None" about ports it never looked for. The caller turns "neither +/// family answered" into `PortProbe::Unavailable` instead. #[cfg(windows)] -fn tcp_table(family: u32) -> Vec { +fn tcp_table(family: u32) -> Option> { use windows_sys::Win32::Foundation::{ERROR_INSUFFICIENT_BUFFER, NO_ERROR}; use windows_sys::Win32::NetworkManagement::IpHelper::{ GetExtendedTcpTable, TCP_TABLE_OWNER_PID_LISTENER, @@ -425,14 +748,14 @@ fn tcp_table(family: u32) -> Vec { ) }; match rc { - NO_ERROR => return buf, + NO_ERROR => return Some(buf), ERROR_INSUFFICIENT_BUFFER => { buf = vec![0u32; (size as usize).div_ceil(std::mem::size_of::()) + 64] } _ => break, } } - Vec::new() + None } /// Where the rows of a `MIB_*TABLE_OWNER_PID` start in `buf`, and how many of @@ -494,11 +817,11 @@ fn spell_v6(addr: std::net::Ipv6Addr) -> String { /// Adds one listening socket to the list, merging it with a row already there /// for the same port and pid. /// -/// The merge rule is the unix path's, for the same reason: a process bound to -/// both `192.168.1.5` and `*` is on localhost, and the row the panel turns into -/// a clickable URL should say so rather than whichever address the kernel -/// happened to list first. -#[cfg(windows)] +/// One rule for both platforms — it was written twice, once inline in the +/// `lsof` parser and once here, and two copies of a merge rule is one too +/// many. A process bound to both `192.168.1.5` and `*` is on localhost, and +/// the row the panel turns into a clickable URL should say so rather than +/// keeping whichever address the kernel or `lsof` happened to list first. fn record_listener(ports: &mut Vec, name: &str, port: u16, pid: u32, addr: String) { if let Some(seen) = ports.iter_mut().find(|e| e.port == port && e.pid == pid) { if !PortEntry::reaches_loopback(&seen.addr) && PortEntry::reaches_loopback(&addr) { @@ -515,8 +838,11 @@ fn record_listener(ports: &mut Vec, name: &str, port: u16, pid: u32, } #[cfg(not(any(unix, windows)))] -fn listening_ports(_procs: &[ProcEntry]) -> Vec { - Vec::new() +fn listening_ports(_procs: &[ProcEntry]) -> (Vec, PortProbe) { + ( + Vec::new(), + PortProbe::Unavailable("no port probe on this platform".to_string()), + ) } /// The address and port `lsof -Fn` reports a listener on — `*:3000`, @@ -538,10 +864,14 @@ fn parse_listen_addr(name: &str) -> Option<(&str, u16)> { mod tests { use super::*; + /// The uid every fabricated row belongs to unless a test says otherwise. + const ME: u32 = 501; + fn row(ppid: u32, name: &str) -> Row { Row { ppid, pgid: 0, + uid: ME, name: name.to_string(), } } @@ -589,7 +919,288 @@ mod tests { .into_iter() .collect(); let got = walk(&table, 100, None); - assert!(got.len() <= MAX_PROCS, "bounded, not infinite"); + assert!(got.len() <= MAX_TREE, "bounded, not infinite"); + } + + /// #731. The walk is depth-first over children in ascending pid order, so + /// a shell whose earlier children brought a crowd used to exhaust the row + /// budget before the traversal reached the newest child — and the newest + /// child, highest pid and visited last, is precisely the `go run` someone + /// started ten seconds ago and is looking for the port of. + #[test] + fn the_newest_child_survives_a_shell_crowded_with_older_ones() { + let mut table: HashMap = [(100, row(1, "zsh"))].into_iter().collect(); + // 80 older children, each with a child of its own: 160 processes, well + // past the 64 the panel draws. + for i in 0..80u32 { + table.insert(200 + i * 2, row(100, "node")); + table.insert(201 + i * 2, row(200 + i * 2, "esbuild")); + } + table.insert(9000, row(100, "go")); + table.insert(9001, row(9000, "main")); + + let walked = walk(&table, 100, None); + assert!( + walked.iter().any(|p| p.pid == 9001 && p.name == "main"), + "the process holding the listener must reach the probe, got {} rows", + walked.len() + ); + + let ports = vec![PortEntry { + port: 8080, + pid: 9001, + addr: "*".into(), + name: "main".into(), + }]; + let out = finish(walked, ports, PortProbe::Ok); + assert_eq!( + out.procs.len(), + MAX_PROCS, + "the panel still gets a short list" + ); + assert_eq!( + out.ports.first().map(|p| p.port), + Some(8080), + "and the port survives the trim that dropped its owner's row" + ); + } + + #[test] + fn a_pathological_tree_still_stops() { + let mut table: HashMap = [(100, row(1, "zsh"))].into_iter().collect(); + for pid in 200..2000u32 { + table.insert(pid, row(100, "fork-bomb")); + } + assert_eq!(walk(&table, 100, None).len(), MAX_TREE); + } + + /// A `sudo go run` is a server the panel can see and a socket it cannot. + /// Saying nothing is listening is the one answer that is certainly wrong. + #[test] + fn another_users_process_in_the_tree_is_noticed() { + let mut table: HashMap = [ + (100, row(1, "zsh")), + (200, row(100, "sudo")), + (300, row(200, "main")), + ] + .into_iter() + .collect(); + let procs = walk(&table, 100, None); + assert!( + !tree_has_foreign_uid(&table, &procs, ME), + "everything is mine until sudo takes over" + ); + + table.get_mut(&200).unwrap().uid = 0; + table.get_mut(&300).unwrap().uid = 0; + assert!(tree_has_foreign_uid(&table, &procs, ME)); + assert!( + !tree_has_foreign_uid(&table, &procs, 0), + "a daemon already running as root is shown everyone's sockets" + ); + } + + /// The `go run` shape from #731, as `lsof -Fpn` reports it: the shell is in + /// the pid list and holds nothing, the compiled binary under `$TMPDIR` + /// holds the listener, and it is bound over both address families. + #[test] + fn parses_a_go_run_report() { + let procs = vec![ + ProcEntry { + pid: 100, + name: "zsh".into(), + depth: 0, + foreground: false, + }, + ProcEntry { + pid: 9000, + name: "go".into(), + depth: 1, + foreground: true, + }, + ProcEntry { + pid: 9001, + name: "main".into(), + depth: 2, + foreground: true, + }, + ]; + let report = "p9001\nf3\nn*:8080\nf5\nn[::]:8080\np100\n"; + let ports = parse_lsof(report, &procs); + assert_eq!( + ports.len(), + 1, + "one server, not one row per family: {ports:?}" + ); + assert_eq!(ports[0].port, 8080); + assert_eq!(ports[0].pid, 9001); + assert_eq!( + ports[0].name, "main", + "the row names the binary `go run` built, not `go`" + ); + assert_eq!(ports[0].authority(), "localhost:8080"); + } + + #[test] + fn a_report_about_nobody_we_asked_about_still_parses() { + // A pid that vanished between the walk and the probe leaves a row with + // no name rather than dropping the port someone can still click. + let ports = parse_lsof("p4242\nn127.0.0.1:5173\n", &[]); + assert_eq!(ports.len(), 1); + assert_eq!(ports[0].name, ""); + } + + #[test] + fn a_quick_probe_comes_back_whole() { + let got = run_bounded(echo_command(), std::time::Duration::from_secs(30)); + let out = match got { + Ok(Run::Finished(ref out)) => out, + ref other => panic!("expected a finished run, got {}", describe(other)), + }; + assert!(String::from_utf8_lossy(&out.stdout).contains("tty7")); + } + + /// The bound is the whole point: this probe runs on the daemon thread that + /// answers `QueryProcs`, and an `lsof` wedged on a dead mount used to own + /// it forever. + /// + /// The sleeper is deliberately a wrapper around a second process, which is + /// the shape that makes this hard: killing the child does not close the + /// pipe its own child inherited, so a `wait_with_output` after the kill + /// would sit on that pipe for the full sleep and hand the caller a timeout + /// that took as long as no timeout at all. + #[test] + fn a_probe_that_never_finishes_is_killed_and_named() { + let cmd = sleeper_command(); + let started = std::time::Instant::now(); + let got = run_bounded(cmd, std::time::Duration::from_millis(300)); + assert!( + matches!(got, Ok(Run::TimedOut)), + "expected a timeout, got {}", + describe(&got) + ); + assert!( + started.elapsed() < std::time::Duration::from_secs(10), + "the caller was released long before the child would have exited" + ); + } + + #[test] + fn a_missing_probe_is_an_error_and_not_an_empty_answer() { + let cmd = std::process::Command::new("tty7-no-such-probe-b0rk"); + assert!( + run_bounded(cmd, std::time::Duration::from_secs(5)).is_err(), + "a tool that is not there has to be distinguishable from one that found nothing" + ); + } + + /// A failure that stands still is one fact, and `snapshot` is asked again + /// every two seconds for as long as the Info panel is open. + #[test] + fn a_standing_probe_failure_is_logged_once_a_minute_not_once_a_poll() { + let mut last = None; + let t0 = std::time::Instant::now(); + let gap = std::time::Duration::from_secs(60); + let line = "shell 100: lsof: program not found"; + assert!( + probe_log_due(&mut last, line, t0, gap), + "the first one talks" + ); + for poll in 1..30u64 { + let now = t0 + std::time::Duration::from_secs(poll * 2); + assert!( + !probe_log_due(&mut last, line, now, gap), + "the same reason again at +{}s", + poll * 2 + ); + } + assert!( + probe_log_due(&mut last, "shell 100: lsof: permission denied", t0, gap), + "a different reason is news even in the same breath" + ); + assert!( + probe_log_due(&mut last, line, t0 + gap, gap), + "and a failure that outlives the gap still leaves a trail" + ); + } + + /// A `/proc/` the kernel handed to root is not evidence of another + /// user: `ping`, and anything else carrying file capabilities, is the + /// caller's own process behind one. + #[test] + fn the_status_files_effective_uid_is_the_one_that_counts() { + let ping = "Name:\tping\nState:\tS (sleeping)\nTgid:\t4242\n\ + Uid:\t501\t501\t501\t501\nGid:\t20\t20\t20\t20\n"; + assert_eq!(status_euid(ping), Some(501)); + + let sudo = "Name:\tmain\nUid:\t501\t0\t0\t0\n"; + assert_eq!( + status_euid(sudo), + Some(0), + "the effective uid, not the real one that typed the password" + ); + + assert_eq!(status_euid("Name:\tzsh\n"), None); + assert_eq!( + status_euid("Uid:\t501\n"), + None, + "a truncated line is no answer" + ); + } + + /// Windows has no tool to be missing, but it does have a kernel call that + /// can refuse, and a refusal used to arrive as an empty table — the same + /// silence #731 is about, on the platform the panel was written on. + #[cfg(windows)] + #[test] + fn a_windows_table_that_would_not_answer_is_not_an_empty_one() { + assert_eq!(windows_probe(true, true), PortProbe::Ok); + assert_eq!( + windows_probe(true, false), + PortProbe::Ok, + "IPv6 switched off is an ordinary machine, not a broken probe" + ); + assert!( + matches!(windows_probe(false, false), PortProbe::Unavailable(_)), + "neither family answered, so the empty list is not an answer" + ); + } + + fn describe(run: &std::io::Result) -> String { + match run { + Ok(Run::Finished(out)) => format!("finished with {}", out.status), + Ok(Run::TimedOut) => "a timeout".to_string(), + Err(e) => format!("an error: {e}"), + } + } + + #[cfg(windows)] + fn echo_command() -> std::process::Command { + let mut c = std::process::Command::new("cmd"); + c.args(["/c", "echo", "tty7"]); + c + } + + #[cfg(not(windows))] + fn echo_command() -> std::process::Command { + let mut c = std::process::Command::new("echo"); + c.arg("tty7"); + c + } + + #[cfg(windows)] + fn sleeper_command() -> std::process::Command { + // `ping` is the sleep every Windows image has. + let mut c = std::process::Command::new("cmd"); + c.args(["/c", "ping", "-n", "20", "127.0.0.1"]); + c + } + + #[cfg(not(windows))] + fn sleeper_command() -> std::process::Command { + let mut c = std::process::Command::new("sleep"); + c.arg("60"); + c } #[test] @@ -779,6 +1390,11 @@ mod windows_tests { "the chain must be walked past its root: {:?}", got.procs ); + assert!( + got.probe.is_ok(), + "a kernel that answered has nothing to apologise for: {:?}", + got.probe + ); let found = got .ports .iter() diff --git a/crates/tty7-core/src/daemon/protocol.rs b/crates/tty7-core/src/daemon/protocol.rs index 0eaa807e..e29d7d4c 100644 --- a/crates/tty7-core/src/daemon/protocol.rs +++ b/crates/tty7-core/src/daemon/protocol.rs @@ -464,10 +464,46 @@ impl PortEntry { } } +/// Whether the answer in `PaneProcs::ports` can be believed. +/// +/// An empty port list used to mean two very different things at once: nothing +/// in this pane is listening, or the thing that looks for listeners never got +/// to say. On unix that look is an `lsof` subprocess, and every way it can go +/// wrong — absent from the daemon's `PATH`, killed, hung on a wedged mount, +/// pointed at sockets it has no permission to read — arrived as the same empty +/// vector as a genuinely quiet pane. Whoever reads the list gets to know which +/// one it is. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "state", content = "detail", rename_all = "snake_case")] +pub enum PortProbe { + /// The probe ran and the list is its whole answer. + #[default] + Ok, + /// The probe ran, but at least one process in this pane belongs to another + /// user — `sudo go run`, a root-owned server on a low port — and a probe + /// running as this user cannot see that process's sockets. The list holds + /// what could be seen, which may be nothing. + Restricted, + /// The probe could not be run at all. The string is for a log line or for + /// `tty7 procs`, not for the panel: it names the tool and what went wrong. + Unavailable(String), +} + +impl PortProbe { + pub fn is_ok(&self) -> bool { + matches!(self, PortProbe::Ok) + } +} + #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct PaneProcs { pub procs: Vec, pub ports: Vec, + /// `serde(default)` because a daemon from before this field existed + /// answers `QueryProcs` without it, and its silence is read the way that + /// daemon's callers read every answer it gives: as a complete one. + #[serde(default)] + pub probe: PortProbe, } fn default_term() -> String { @@ -2192,6 +2228,35 @@ mod tests { assert!(ClientMsg::read(&mut empty2).is_err()); } + /// The port probe's verdict travels over the same wire as the ports, and + /// the two ends of that wire are regularly different builds: a remote + /// workspace's panes are answered for by whatever `tty7-server` is + /// installed on the peer. An older one says nothing about the probe, and + /// its silence has to read as "this list is the whole answer" rather than + /// failing the frame or, worse, arriving as a doubt the panel then shows. + #[test] + fn a_procs_answer_without_a_probe_verdict_is_a_complete_one() { + let old = r#"{"procs":[],"ports":[{"port":3000,"pid":7,"name":"node"}]}"#; + let procs: PaneProcs = serde_json::from_str(old).unwrap(); + assert_eq!(procs.probe, PortProbe::Ok); + assert!(procs.probe.is_ok()); + assert_eq!(procs.ports[0].addr, ""); + + for probe in [ + PortProbe::Ok, + PortProbe::Restricted, + PortProbe::Unavailable("lsof: not found".into()), + ] { + let wire = serde_json::to_string(&PaneProcs { + probe: probe.clone(), + ..Default::default() + }) + .unwrap(); + let back: PaneProcs = serde_json::from_str(&wire).unwrap(); + assert_eq!(back.probe, probe, "round trip through {wire}"); + } + } + #[test] fn pane_info_deserializes_with_defaults() { let info: PaneInfo = serde_json::from_str(r#"{"pane_id": 5, "alive": true}"#).unwrap(); diff --git a/crates/tty7-core/src/host/server.rs b/crates/tty7-core/src/host/server.rs index 1f2fa862..9db770c8 100644 --- a/crates/tty7-core/src/host/server.rs +++ b/crates/tty7-core/src/host/server.rs @@ -1711,6 +1711,7 @@ mod aggregate_tests { name: "node".into(), addr: "*".into(), }], + probe: Default::default(), } } diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index f3572312..4884935d 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -1066,6 +1066,10 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::PanelProcessesSubtitle => "Processes", L10nKey::PanelPortsSubtitle => "Ports", L10nKey::PanelPortsUnsupported => "That machine's tty7-server is too old to list ports.", + L10nKey::PanelPortsProbeFailed => "Couldn't check what this pane is listening on.", + L10nKey::PanelPortsRestricted => { + "Something here runs as another user, whose ports aren't visible." + } L10nKey::PortAutoForwarded => "Remote :{port} is now http://localhost:{local}", L10nKey::PanelCwd => "cwd", L10nKey::PanelShell => "shell", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index b0aa4296..d4e30d71 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -1130,6 +1130,12 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::PanelProcessesSubtitle => "プロセス", L10nKey::PanelPortsSubtitle => "ポート", L10nKey::PanelPortsUnsupported => "リモートの tty7-server が古く、ポートを列挙できません。", + L10nKey::PanelPortsProbeFailed => { + "このペインが何をリッスンしているか確認できませんでした。" + } + L10nKey::PanelPortsRestricted => { + "他のユーザーで動いているプロセスがあり、そのポートは見えません。" + } L10nKey::PortAutoForwarded => "リモートの :{port} は http://localhost:{local} で開けます", L10nKey::PanelCwd => "作業ディレクトリ", L10nKey::PanelShell => "シェル", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index ec9e2ac3..627f7c2c 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -786,6 +786,8 @@ l10n_keys! { PanelProcessesSubtitle, PanelPortsSubtitle, PanelPortsUnsupported, + PanelPortsProbeFailed, + PanelPortsRestricted, PortAutoForwarded, PanelCwd, PanelShell, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index f4b90a1e..8b53c7a7 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -1021,6 +1021,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::PanelProcessesSubtitle => "进程", L10nKey::PanelPortsSubtitle => "端口", L10nKey::PanelPortsUnsupported => "对端的 tty7-server 太旧，列不出端口。", + L10nKey::PanelPortsProbeFailed => "没能查出这个窗格在监听什么。", + L10nKey::PanelPortsRestricted => "这里有以其他用户身份运行的进程，看不到它们的端口。", L10nKey::PortAutoForwarded => "远程 :{port} 现在是 http://localhost:{local}", L10nKey::PanelCwd => "工作目录", L10nKey::PanelShell => "shell", diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index 10580ae9..43e21b74 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -8,7 +8,7 @@ use gpui_component::{ use std::path::PathBuf; use crate::core::config::{Config, RightPanelTab}; -use crate::daemon::protocol::{ManagedForward, PaneProcs}; +use crate::daemon::protocol::{ManagedForward, PaneProcs, PortProbe}; use crate::ui::app::{ CONTENT_INSET, TILE_GLYPH_SM, TILE_GLYPH_XS, TILE_SIZE_SM, TILE_SIZE_XS, Tty7App, tile_trailing_inset, tile_trailing_inset_sm, @@ -1149,9 +1149,12 @@ impl Tty7App { ) -> Option { let ctx = ctx?; let pane_id = ctx.pane_id; - let ports = self + // No answer yet for this pane defaults to a probe that is fine, not a + // broken one: the panel has nothing to doubt until it has been told + // something. + let (ports, probe) = self .procs(Some(pane_id)) - .map(|p| p.ports.clone()) + .map(|p| (p.ports.clone(), p.probe.clone())) .unwrap_or_default(); let forwards: Vec = self .loopback_panel @@ -1163,7 +1166,13 @@ impl Tty7App { let form_open = self.loopback_panel.form_pane_id == Some(pane_id); // A pane that cannot hold a forward and is serving nothing has no // section: the heading alone would be an empty promise. - if ports.is_empty() && forwards.is_empty() && ctx.route.is_none() { + // + // Unless the reason it is serving nothing is that nobody managed to + // look. Then the heading and one muted line under it are the only + // place the panel can admit it does not know, and a silently absent + // section is the bug (#731): someone whose server is plainly up reads + // the missing section as tty7 saying there is no server. + if ports.is_empty() && forwards.is_empty() && ctx.route.is_none() && probe.is_ok() { return None; } @@ -1324,22 +1333,26 @@ impl Tty7App { |this| { // "Nothing is listening" and "nobody could tell us" // look identical on screen unless the panel says which - // one it means — and the second is a fixable thing: - // the far end is running a server too old to answer. - let (text, tone) = match self.right_panel.procs_unsupported { - true => ( - t(L10nKey::PanelPortsUnsupported), - cx.theme().muted_foreground, - ), - false => (t(L10nKey::None), cx.theme().muted_foreground), + // one it means — and every one of the second kind is a + // fixable thing: a far end running a server too old to + // answer, a probe that could not be run at all, a + // server started under `sudo` whose sockets this user + // is not allowed to see. Still one muted line in the + // place the word "None" would have gone; the panel is + // reporting what it knows, not raising an alarm. + let key = match (self.right_panel.procs_unsupported, &probe) { + (true, _) => L10nKey::PanelPortsUnsupported, + (false, PortProbe::Unavailable(_)) => L10nKey::PanelPortsProbeFailed, + (false, PortProbe::Restricted) => L10nKey::PanelPortsRestricted, + (false, PortProbe::Ok) => L10nKey::None, }; this.child( div() .px(px(CONTENT_INSET)) .py(px(2.)) .text_size(rems(TEXT)) - .text_color(tone) - .child(text), + .text_color(cx.theme().muted_foreground) + .child(t(key)), ) }, )