From 29312d5bb8d47cf46965a09b0310dc55c1d7db22 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 9 Sep 2026 18:13:51 +0800 Subject: [PATCH 1/2] fix(procinfo): stop the listening-port probe failing silently (#731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Ports section says nothing when a pane has no listeners, and it said exactly the same thing when the code that looks for listeners never ran. #731 is a report from inside that gap: a Go service started with `go run main.go` on macOS serves requests, and tty7 shows no port. The reported shape itself holds up. `snapshot()` walks the whole descendant tree from the pane's shell, `go run`'s compiled binary sits at depth 2, and the `lsof` invocation is the one that finds it. What did not hold up is everything around that. The walk stopped at 64 processes, and it is depth-first over children in ascending pid order. A shell whose earlier children brought a crowd — a build, a container runtime, an agent's worker pool — could spend the whole budget before the traversal reached the newest child, and the newest child, highest pid and visited last, is precisely the server someone started ten seconds ago. The walk now runs to a far larger bound and the probe is asked about all of it; only the list handed to the panel is cut back to 64 rows. Every way the probe can fail arrived as the same empty vector. `lsof` missing from the daemon's PATH read as "nothing is listening" — and the daemon's PATH is not the shell's, while macOS keeps `lsof` in /usr/sbin, the sort of entry a hand-written `export PATH=...` drops. So did a probe that hung: `Command::output` has no deadline and this runs on the thread answering `QueryProcs`, so one `lsof` wedged on a dead mount takes the whole pane's process list with it, permanently. The probe now falls back to the absolute paths, is bounded at three seconds, and says which of those happened. A server under `sudo` is visible as a process and invisible as a socket: `lsof` running as this user cannot read another user's fds. The walk now carries each process's effective uid, and a tree holding someone else's process says so rather than claiming the pane is quiet. `PaneProcs` grew a `probe` verdict, `serde(default)` so an older `tty7-server` at the far end of a remote workspace still parses and its silence still reads as a complete answer. The panel spends it on the one muted line where it used to write "None", and `tty7 procs` — the command the issue asks reporters to run — prints a note under the empty PORTS table. No banner and no new colour: an honest empty state, not a warning. Deliberately left alone: `lsof`'s exit status. It returns 1 for a pid it could not locate, and a pane's tree loses processes between the walk and the probe as a matter of course, so reading that as a broken probe would put a doubt on screen every time a command finished. A non-zero exit that also found nothing gets a debug log line and no more. The Windows path is untouched beyond its new return type — `GetExtendedTcpTable` has no tool to be missing and no subprocess to hang. I could not reproduce #731, and none of these is proven to be the reporter's bug. Each is a way the panel could be silently wrong, and the verdict is what will make the next report say which one. Claude-Session: https://claude.ai/code/session_01UUyWQXzcBAoBzaSX8pc7nU --- crates/tty7-cli/src/commands.rs | 2 + crates/tty7-cli/src/output.rs | 84 +++- crates/tty7-core/src/daemon/procinfo.rs | 522 +++++++++++++++++++++--- crates/tty7-core/src/daemon/protocol.rs | 65 +++ crates/tty7-core/src/host/server.rs | 1 + src/ui/i18n/en.rs | 4 + src/ui/i18n/ja.rs | 6 + src/ui/i18n/mod.rs | 2 + src/ui/i18n/zh.rs | 2 + src/ui/right_panel.rs | 41 +- 10 files changed, 661 insertions(+), 68 deletions(-) diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index c31f2c1d..931a5e67 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -2881,6 +2881,7 @@ mod tests { tty7_core::daemon::protocol::PaneProcs { procs: vec![proc_entry(100, "zsh", 0, true)], ports: Vec::new(), + probe: Default::default(), } } @@ -2892,6 +2893,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..c937ec37 100644 --- a/crates/tty7-core/src/daemon/procinfo.rs +++ b/crates/tty7-core/src/daemon/procinfo.rs @@ -1,21 +1,95 @@ 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 { + log::warn!("listening-port probe failed for pane shell {shell_pid}: {detail}"); + } + finish(procs, ports, probe) +} + +/// 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)) +} + +#[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. + uid: u32, name: String, } @@ -32,7 +106,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 +170,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 +224,23 @@ 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 owned by the process's effective uid, which is the + // one that governs who may read its sockets. 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 +255,7 @@ fn process_table() -> HashMap { Row { ppid: p.parent, pgid: 0, + uid: 0, name: p.name, }, ) @@ -221,20 +316,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 +359,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 +479,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 +509,12 @@ 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. +/// +/// The probe state is always `Ok` here. There is no tool to be missing and no +/// subprocess to hang: `GetExtendedTcpTable` either answers or the family is +/// skipped, and a machine with IPv6 off still gets its IPv4 ports. #[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,7 +523,7 @@ 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(); @@ -378,7 +580,7 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { } ports.sort_by_key(|e| (e.port, e.pid)); - ports + (ports, PortProbe::Ok) } /// The two Winsock address families, named here rather than by switching on @@ -494,11 +696,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 +717,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 +743,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 +798,216 @@ 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" + ); + } + + 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] 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 42b04b3d..6d5aafca 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -1073,6 +1073,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 6560bf5a..f1c1c41a 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -1137,6 +1137,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 0cc6d32d..3d51402f 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -789,6 +789,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 83d55ae5..eee42a1e 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -1024,6 +1024,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 da037237..15689140 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, @@ -1248,9 +1248,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 @@ -1262,7 +1265,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; } @@ -1423,22 +1432,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)), ) }, ) From 16b663a407ab23cce66d5ec5f7245000ff518223 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 10 Sep 2026 14:02:08 +0800 Subject: [PATCH 2/2] fix(procinfo): report a Windows table that would not answer, and stop crying wolf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things the new probe verdict still got wrong. `GetExtendedTcpTable` refusing came back as an empty buffer, which read exactly like a family with no listeners, so the Windows branch answered `Ok` about ports it had never looked for — the silence #731 is about, on the platform the panel was written on. `tcp_table` says `None` now, and only "neither family answered" is `Unavailable`: a machine with IPv6 off keeps its `Ok` and its IPv4 ports. The `Unavailable` warning was written on every `QueryProcs`, and the Info panel sends one every two seconds — thirty identical lines a minute into a log that truncates itself at 4 MiB, which costs a reporter the rest of the session they turned logging on to capture. Same reason, once a minute. And `Restricted` on Linux took `/proc/`'s owner for the process's uid. The kernel hands that directory to root whenever it clears a process's dumpable attribute, which is what executing a set-user-ID binary or one carrying file capabilities does, so a plain `ping` in a pane had the panel apologising for sockets it can read perfectly well — the opposite mistake, and just as wrong. The `Uid:` line of `/proc//status` settles the few rows that look foreign and are in a pane's tree, so an ordinary pane pays nothing for it. Claude-Session: https://claude.ai/code/session_01JRqYZ9E153WpSHGS2AW3BM --- crates/tty7-core/src/daemon/procinfo.rs | 256 +++++++++++++++++++++--- 1 file changed, 227 insertions(+), 29 deletions(-) diff --git a/crates/tty7-core/src/daemon/procinfo.rs b/crates/tty7-core/src/daemon/procinfo.rs index c937ec37..04897d8e 100644 --- a/crates/tty7-core/src/daemon/procinfo.rs +++ b/crates/tty7-core/src/daemon/procinfo.rs @@ -29,11 +29,52 @@ pub fn snapshot(shell_pid: u32, fg_pgid: Option) -> PaneProcs { false => probe, }; if let PortProbe::Unavailable(detail) = &probe { - log::warn!("listening-port probe failed for pane shell {shell_pid}: {detail}"); + 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. /// @@ -64,9 +105,58 @@ fn tree_has_foreign_uid(table: &HashMap, procs: &[ProcEntry], me: u32) if me == 0 { return false; } - procs - .iter() - .any(|p| table.get(&p.pid).is_some_and(|row| row.uid != me)) + 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)] @@ -88,7 +178,9 @@ 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. + /// 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, } @@ -224,11 +316,14 @@ fn process_table() -> HashMap { .rfind('(') .map_or_else(|| String::new(), |open| stat[open + 1..close].to_string()) }); - // `/proc/` is owned by the process's effective uid, which is the - // one that governs who may read its sockets. 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. + // `/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()); @@ -510,9 +605,12 @@ fn parse_lsof(text: &str, procs: &[ProcEntry]) -> Vec { /// seconds while it is open, so this is a fixed handful of microseconds, with /// no process spawn and nothing allocated per pid. /// -/// The probe state is always `Ok` here. There is no tool to be missing and no -/// subprocess to hang: `GetExtendedTcpTable` either answers or the family is -/// skipped, and a machine with IPv6 off still gets its IPv4 ports. +/// 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, PortProbe) { use std::net::{Ipv4Addr, Ipv6Addr}; @@ -529,12 +627,13 @@ fn listening_ports(procs: &[ProcEntry]) -> (Vec, PortProbe) { 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); @@ -559,9 +658,9 @@ fn listening_ports(procs: &[ProcEntry]) -> (Vec, PortProbe) { 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 { @@ -580,7 +679,25 @@ fn listening_ports(procs: &[ProcEntry]) -> (Vec, PortProbe) { } ports.sort_by_key(|e| (e.port, e.pid)); - (ports, PortProbe::Ok) + (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 @@ -592,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, @@ -627,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 @@ -973,6 +1094,78 @@ mod tests { ); } + /// 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), @@ -1197,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()