From bedbe0a2ecde02469059f781beaa1df0b35d15cd Mon Sep 17 00:00:00 2001 From: jasonwitty Date: Mon, 24 Aug 2026 07:19:37 -0700 Subject: [PATCH] Add flag to override logic and supress terminate option. (--no-kill) Flag specifically used to block feature on socktop.io. Will remain undocumented for standard usage. --- CHANGELOG.md | 12 ++++++++ socktop/src/app.rs | 28 +++++++++--------- socktop/src/main.rs | 46 ++++++++++++++++++++++-------- socktop/src/ui/modal_process.rs | 4 +-- socktop/src/ui/modal_types.rs | 7 +++-- socktop/src/ui/processes.rs | 15 +++++----- socktop/tests/cli_args.rs | 50 +++++++++++++++++++++++++++++++++ 7 files changed, 126 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c52bf7..da44ad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## Unreleased + +### TUI + +- **`--no-kill` flag and `SOCKTOP_NO_KILL` env var** disable the local + process-kill feature regardless of agent locality, for shared terminals and + public demos (e.g. the socktop.io webterm). Either one forces the feature + off and suppresses the `t` kill hints; the env var covers every socktop + invocation under a deployment without touching command lines. `App`'s + builder renamed `with_local` → `with_kill_enabled` to match what it now + means (locality fact AND policy). + ## 1.60.1 — unreleased Identical to 1.60.0 plus rebuilt Debian packages: the 1.60.0 debs were linked diff --git a/socktop/src/app.rs b/socktop/src/app.rs index 24a8501..3f2c54d 100644 --- a/socktop/src/app.rs +++ b/socktop/src/app.rs @@ -194,9 +194,10 @@ pub struct App { // Security / status flags pub is_tls: bool, pub has_token: bool, - // Whether the connected agent is on this machine. Gates the local - // process-kill feature (t = SIGTERM, k = SIGKILL). - pub is_local: bool, + // Whether the local process-kill feature (t = SIGTERM, k = SIGKILL) is + // available: the connected agent is on this machine AND no policy override + // (--no-kill / SOCKTOP_NO_KILL) has disabled it. + pub kill_enabled: bool, // Pending kill awaiting confirmation: (pid, process name). Which signal is // sent depends on the button chosen in the confirmation modal, so it isn't // decided until then. @@ -295,7 +296,7 @@ impl App { verify_hostname: false, is_tls: false, has_token: false, - is_local: false, + kill_enabled: false, pending_kill: None, force_compact: false, header_title: String::new(), @@ -349,9 +350,10 @@ impl App { } /// Enable the local process-kill feature. Only set true when the agent has - /// been verified to be on this machine (see [`crate::local`]). - pub fn with_local(mut self, is_local: bool) -> Self { - self.is_local = is_local; + /// been verified to be on this machine (see [`crate::local`]) and no + /// policy override (`--no-kill`, `SOCKTOP_NO_KILL`) forbids it. + pub fn with_kill_enabled(mut self, kill_enabled: bool) -> Self { + self.kill_enabled = kill_enabled; self } @@ -375,11 +377,11 @@ impl App { .map(|p| p.name.clone()) } - /// Raise the kill confirmation for `pid`. No-op unless the agent is on this - /// machine — the same gate the keybinding uses, repeated here because this + /// Raise the kill confirmation for `pid`. No-op unless the kill feature is + /// enabled — the same gate the keybinding uses, repeated here because this /// is also reachable from the details modal. fn prompt_kill(&mut self, pid: u32) { - if !self.is_local { + if !self.kill_enabled { return; } let name = self @@ -1182,7 +1184,7 @@ impl App { // the details modal so it could not be reused there, and one // key for both entry points is one thing to remember. // SIGTERM vs SIGKILL is chosen in the confirmation modal. - if self.is_local + if self.kill_enabled && !self.modal_manager.is_active() && matches!(k.code, KeyCode::Char('t') | KeyCode::Char('T')) && let Some(pid) = self.selected_process_pid @@ -1948,7 +1950,7 @@ impl App { filtered_indices: &self.procs_filtered, cached_rows: &self.procs_row_cache, peak_cpu: self.procs_row_peak_cpu, - is_local: self.is_local, + kill_enabled: self.kill_enabled, }, ); @@ -1969,7 +1971,7 @@ impl App { }, max_mem_bytes: self.max_process_mem_bytes, unsupported: self.process_details_unsupported, - is_local: self.is_local, + kill_enabled: self.kill_enabled, }, ); } diff --git a/socktop/src/main.rs b/socktop/src/main.rs index a7ffca4..f8a4792 100644 --- a/socktop/src/main.rs +++ b/socktop/src/main.rs @@ -25,6 +25,18 @@ pub(crate) struct ParsedArgs { processes_interval_ms: Option, verify_hostname: bool, compact: bool, + no_kill: bool, +} + +/// True when the `SOCKTOP_NO_KILL` environment variable disables the process-kill +/// feature. Any value other than empty, `0`, or `false` (case-insensitive) counts +/// as set, so a deployment can export `SOCKTOP_NO_KILL=1` once and every socktop +/// launched under it — whatever its command line — has the feature off. +pub(crate) fn no_kill_from_env() -> bool { + match env::var("SOCKTOP_NO_KILL") { + Ok(v) => !v.is_empty() && v != "0" && !v.eq_ignore_ascii_case("false"), + Err(_) => false, + } } pub(crate) fn parse_args>(args: I) -> Result { @@ -40,11 +52,12 @@ pub(crate) fn parse_args>(args: I) -> Result = None; let mut verify_hostname = false; let mut compact = false; + let mut no_kill = false; while let Some(arg) = it.next() { match arg.as_str() { "-h" | "--help" => { return Err(format!( - "Usage: {prog} [--tls-ca CERT_PEM|-t CERT_PEM] [--verify-hostname] [--profile NAME|-P NAME] [--save] [--demo] [--compact] [--metrics-interval-ms N] [--processes-interval-ms N] [ws://HOST:PORT/ws]\n" + "Usage: {prog} [--tls-ca CERT_PEM|-t CERT_PEM] [--verify-hostname] [--profile NAME|-P NAME] [--save] [--demo] [--compact] [--no-kill] [--metrics-interval-ms N] [--processes-interval-ms N] [ws://HOST:PORT/ws]\n" )); } "--tls-ca" | "-t" => { @@ -70,6 +83,12 @@ pub(crate) fn parse_args>(args: I) -> Result { + // Disable the local process-kill feature even when the agent is + // local. For shared/kiosk deployments; SOCKTOP_NO_KILL=1 in the + // environment does the same without touching the command line. + no_kill = true; + } "--dry-run" => { // intentionally undocumented dry_run = true; @@ -109,7 +128,7 @@ pub(crate) fn parse_args>(args: I) -> Result>(args: I) -> Result Result<(), Box> { } if parsed.demo || matches!(parsed.profile.as_deref(), Some("demo")) { - return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact).await; + return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact, parsed.no_kill).await; } let profiles_file = load_profiles(); @@ -251,7 +271,7 @@ async fn main() -> Result<(), Box> { if (1..=names.len()).contains(&idx) { let name = &names[idx - 1]; if name == "demo" { - return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact).await; + return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact, parsed.no_kill).await; } if let Some(entry) = profiles_mut.profiles.get(name) { ( @@ -311,7 +331,7 @@ async fn main() -> Result<(), Box> { ); eprintln!("If you don't have an agent running, you can try the demo mode."); if prompt_yes_no("Would you like to start the demo mode now? [Y/n]: ") { - return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact).await; + return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact, parsed.no_kill).await; } else { eprintln!("Aborting. You can run 'socktop --help' for usage information."); return Ok(()); @@ -324,14 +344,16 @@ async fn main() -> Result<(), Box> { let is_tls = url.starts_with("wss://"); let has_token = url.contains("token="); // Only enable local process-kill when the agent is verified to be on this - // machine; otherwise on-screen PIDs refer to a remote host and acting on - // them locally would signal the wrong process. See local::agent_is_local. - let is_local = local::agent_is_local(&url); + // machine — otherwise on-screen PIDs refer to a remote host and acting on + // them locally would signal the wrong process (see local::agent_is_local) — + // AND neither --no-kill nor SOCKTOP_NO_KILL disables it as a matter of + // policy (shared terminals, public demos). + let kill_enabled = local::agent_is_local(&url) && !parsed.no_kill && !no_kill_from_env(); let mut app = App::new() .with_intervals(metrics_interval_ms, processes_interval_ms) .with_status(is_tls, has_token) .with_compact(parsed.compact) - .with_local(is_local); + .with_kill_enabled(kill_enabled); if parsed.dry_run { return Ok(()); } @@ -398,6 +420,7 @@ fn gather_intervals( async fn run_demo_mode( _tls_ca: Option<&str>, compact: bool, + no_kill: bool, ) -> Result<(), Box> { let port = 3231; let url = format!("ws://127.0.0.1:{port}/ws"); @@ -413,10 +436,11 @@ async fn run_demo_mode( }; // Demo mode runs the real agent on loopback, so its PIDs are real local // processes — enable the local process-kill feature, gated the same way as - // the normal connect path (loopback resolves local). + // the normal connect path (loopback resolves local, --no-kill and + // SOCKTOP_NO_KILL still override). let mut app = App::new() .with_compact(compact) - .with_local(local::agent_is_local(&url)); + .with_kill_enabled(local::agent_is_local(&url) && !no_kill && !no_kill_from_env()); // Demo mode connects to localhost, so disable hostname verification tokio::select! { res=app.run(&url,None,false)=>{ drop(child); res } _=tokio::signal::ctrl_c()=>{ drop(child); Ok(()) } } } diff --git a/socktop/src/ui/modal_process.rs b/socktop/src/ui/modal_process.rs index 56aa034..f4366d1 100644 --- a/socktop/src/ui/modal_process.rs +++ b/socktop/src/ui/modal_process.rs @@ -130,8 +130,8 @@ impl ModalManager { ])]; // Kill from here too — same key as the processes pane, and only shown - // when the agent is local, since that is the only case where it works. - if data.is_local + // when the kill feature is enabled (agent local, no policy override). + if data.kill_enabled && let Some(line) = help_text.first_mut() { line.spans.push(Span::styled( diff --git a/socktop/src/ui/modal_types.rs b/socktop/src/ui/modal_types.rs index a45a85d..7be040a 100644 --- a/socktop/src/ui/modal_types.rs +++ b/socktop/src/ui/modal_types.rs @@ -19,9 +19,10 @@ pub struct ProcessModalData<'a> { pub history: ProcessHistoryData<'a>, pub max_mem_bytes: u64, pub unsupported: bool, - /// Whether the agent is on this machine. Only used to decide whether the - /// `t` kill hint is shown — the kill itself is gated in `App`. - pub is_local: bool, + /// Whether the process-kill feature is available (agent local, no policy + /// override). Only used to decide whether the `t` kill hint is shown — + /// the kill itself is gated in `App`. + pub kill_enabled: bool, } /// Parameters for rendering scatter plot diff --git a/socktop/src/ui/processes.rs b/socktop/src/ui/processes.rs index 0931e88..9be45cb 100644 --- a/socktop/src/ui/processes.rs +++ b/socktop/src/ui/processes.rs @@ -87,9 +87,10 @@ pub struct ProcessDisplayParams<'a> { /// Peak cpu_usage from the most recent cache build; used to bold the /// busiest process. -1.0 if no cache. pub peak_cpu: f32, - /// Agent is on this machine, so the `t` kill hint applies. Without it the - /// hint would advertise a key that deliberately does nothing. - pub is_local: bool, + /// The process-kill feature is available (agent local, no policy + /// override), so the `t` kill hint applies. Without it the hint would + /// advertise a key that deliberately does nothing. + pub kill_enabled: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -464,7 +465,7 @@ pub fn draw_top_processes(f: &mut ratatui::Frame<'_>, area: Rect, params: Proces Span::styled(" details", label), Span::styled(" · ", label), ]; - if params.is_local { + if params.kill_enabled { hints.push(Span::styled("t", key)); hints.push(Span::styled(" kill", label)); hints.push(Span::styled(" · ", label)); @@ -952,7 +953,7 @@ mod click_tests { filtered_indices: &idxs, cached_rows: &cache, peak_cpu: peak, - is_local: false, + kill_enabled: false, }, ) }) @@ -1066,7 +1067,7 @@ mod tooltip_tests { } /// Render the pane with a selection and return the whole buffer as text. - fn rendered(name: &str, width: u16, is_local: bool) -> String { + fn rendered(name: &str, width: u16, kill_enabled: bool) -> String { let m = metrics(name); let mut cache = Vec::new(); let peak = rebuild_row_cache(&m, &mut cache); @@ -1088,7 +1089,7 @@ mod tooltip_tests { filtered_indices: &idxs, cached_rows: &cache, peak_cpu: peak, - is_local, + kill_enabled, }, ) }) diff --git a/socktop/tests/cli_args.rs b/socktop/tests/cli_args.rs index f767307..6118a69 100644 --- a/socktop/tests/cli_args.rs +++ b/socktop/tests/cli_args.rs @@ -106,3 +106,53 @@ fn test_compact_flag_documented_and_accepted() { String::from_utf8_lossy(&out2.stderr) ); } + +#[test] +fn test_no_kill_flag_documented_and_accepted() { + let exe = env!("CARGO_BIN_EXE_socktop"); + let out = Command::new(exe) + .args(["--no-kill", "--help"]) + .output() + .expect("run socktop --no-kill --help"); + assert!( + out.status.success(), + "socktop --no-kill --help did not succeed" + ); + let text = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!( + text.contains("--no-kill"), + "help text missing --no-kill\n{text}" + ); + + // The flag must not be mistaken for the positional URL argument. + let out2 = Command::new(exe) + .args(["--no-kill", "--dry-run", "ws://127.0.0.1:3000/ws"]) + .output() + .expect("run socktop --no-kill --dry-run"); + assert!( + out2.status.success(), + "socktop --no-kill with a URL was rejected: {}", + String::from_utf8_lossy(&out2.stderr) + ); +} + +#[test] +fn test_no_kill_env_var_accepted() { + // SOCKTOP_NO_KILL must not break startup — the env-only path is how the + // webterm deployment disables the kill feature for every invocation. + let exe = env!("CARGO_BIN_EXE_socktop"); + let out = Command::new(exe) + .env("SOCKTOP_NO_KILL", "1") + .args(["--dry-run", "ws://127.0.0.1:3000/ws"]) + .output() + .expect("run socktop with SOCKTOP_NO_KILL=1"); + assert!( + out.status.success(), + "socktop with SOCKTOP_NO_KILL=1 did not succeed: {}", + String::from_utf8_lossy(&out.stderr) + ); +}