diff --git a/Cargo.lock b/Cargo.lock index 950fd9d..6b91db4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -306,11 +306,11 @@ dependencies = [ [[package]] name = "serde_spanned" -version = "0.6.9" +version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +checksum = "6662b5879511e06e8999a8a235d848113e942c9124f211511b16466ee2995f26" dependencies = [ - "serde", + "serde_core", ] [[package]] @@ -352,44 +352,42 @@ dependencies = [ [[package]] name = "toml" -version = "0.8.23" +version = "1.1.4+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" -dependencies = [ - "serde", - "serde_spanned", - "toml_datetime", - "toml_edit", -] - -[[package]] -name = "toml_datetime" -version = "0.6.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" -dependencies = [ - "serde", -] - -[[package]] -name = "toml_edit" -version = "0.22.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +checksum = "3aace63f4bbcdfc2c965b059de67119c89c4017a70d633be6c104910f67056f5" dependencies = [ "indexmap", - "serde", + "serde_core", "serde_spanned", "toml_datetime", - "toml_write", + "toml_parser", + "toml_writer", "winnow", ] [[package]] -name = "toml_write" -version = "0.1.2" +name = "toml_datetime" +version = "1.1.1+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" +checksum = "3165f65f62e28e0115a00b2ebdd37eb6f3b641855f9d636d3cd4103767159ad7" +dependencies = [ + "serde_core", +] + +[[package]] +name = "toml_parser" +version = "1.1.3+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d38ac1cf9b95face32296c0a3ede1fdc270627c9d9c02a7274dd6d960dc4d56" +dependencies = [ + "winnow", +] + +[[package]] +name = "toml_writer" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" [[package]] name = "unicode-ident" @@ -426,12 +424,9 @@ dependencies = [ [[package]] name = "winnow" -version = "0.7.15" +version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" -dependencies = [ - "memchr", -] +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" [[package]] name = "zmij" diff --git a/Cargo.toml b/Cargo.toml index a34e85c..dd7858d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ dirs = "6" serde = { version = "1", features = ["derive"] } serde_json = "1" -toml = "0.8" +toml = "^1" libc = "0.2" diff --git a/spec/porthole-spec.md b/spec/porthole-spec.md index 4767ac7..c98c30a 100644 --- a/spec/porthole-spec.md +++ b/spec/porthole-spec.md @@ -204,7 +204,7 @@ Saves a new profile. Does **not** open it. | `-l, --local` | `[bind:]port:host:hostport` | one of `-l/-r/-d` | — | Local forward: your machine → remote | | `-r, --remote` | `[bind:]port:host:hostport` | one of `-l/-r/-d` | — | Remote forward: remote → your machine | | `-d, --dynamic` | `[bind:]port` | one of `-l/-r/-d` | — | Dynamic forward (SOCKS proxy) | -| `--via` | `[user@]host[:port][,...]` | **yes** | — | Comma-separated hop chain; the *last* hop is the actual `ssh` connection target, any before it are `-J` jumps (§3.1) | +| `--via` | `[user@]host[:port]` | **yes** | — | One hop chain entry; repeatable (`--via a --via b`) and/or comma-separated (`--via a,b`) - the last hop is the `ssh` connection target, any before it are `-J` jumps (§3.1) | | `-u, --user` | `user` | no | current user / ssh_config | Default user for the final target and any `--via` hop that doesn't specify its own | | `-i, --identity` | `path` | no | ssh_config default | | | `-p, --port` | `port` | no | `22` | SSH port on the final target only — a `--via` hop needs its own inline `:port` if it isn't 22 | diff --git a/src/atomic.rs b/src/atomic.rs index a8556e1..e660b6d 100644 --- a/src/atomic.rs +++ b/src/atomic.rs @@ -7,7 +7,8 @@ use std::io::Write; use std::path::Path; -pub fn write(path: &Path, contents: &[u8]) -> std::io::Result<()> { +pub fn write(path: &Path, contents: &[u8]) -> std::io::Result<()> +{ let tmp = path.with_extension(format!( "{}.tmp.{}", path.extension().and_then(|e| e.to_str()).unwrap_or(""), diff --git a/src/cli.rs b/src/cli.rs index 21caa94..1e2ef9a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -54,24 +54,23 @@ pub enum Commands { /// struct (`#[command(flatten)]`ed into both) so the two can never drift. #[derive(Args, Default)] pub struct MappingArgs { - /// Local forward: your machine -> remote. [bind:]port:host:hostport - #[arg(short, long, value_name = "SPEC")] + /// Local forward: your machine -> remote. + #[arg(short, long, value_name = "[BIND:]PORT:HOST:PORT")] pub local: Option, - /// Remote forward: remote -> your machine. [bind:]port:host:hostport - #[arg(short, long, value_name = "SPEC")] + /// Remote forward: remote -> your machine. + #[arg(short, long, value_name = "[BIND:]PORT:HOST:PORT")] pub remote: Option, - /// Dynamic forward (SOCKS proxy). [bind:]port - #[arg(short, long, value_name = "SPEC")] + /// Dynamic forward (SOCKS proxy). + #[arg(short, long, value_name = "[BIND:]PORT")] pub dynamic: Option, - /// SSH hop chain, comma-separated: [user@]host[:port][,...]. The last - /// hop is the actual connection target, any before it are -J jumps. - #[arg(long, value_name = "HOPS")] - pub via: Option, + /// SSH hop chain: jump hosts plus the final connection target. + #[arg(long, value_name = "[USER@]HOST[:PORT]", value_delimiter = ',')] + pub via: Vec, - /// Default user for the target and any --via hop without its own. + /// Default user for the target and any hop without one. #[arg(short, long, value_name = "USER")] pub user: Option, diff --git a/src/commands/mod.rs b/src/commands/mod.rs index a2c7738..32743b8 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -11,12 +11,13 @@ pub mod wipe; use crate::cli::MappingArgs; use crate::profile::ProfileEdits; -/// Turns clap's `MappingArgs` into a `ProfileEdits`, splitting `--via`'s -/// comma-separated raw string into the ordered hop list (spec §5.1). +/// Turns clap's `MappingArgs` into a `ProfileEdits` (spec §5.1). `--via` is +/// collected by clap itself: `value_delimiter = ','` splits each +/// occurrence on commas, and the field being a `Vec` allows repeated +/// `--via` flags, so both `--via a,b` and `--via a --via b` reach here as +/// `["a", "b"]`. pub fn edits_from_mapping(m: &MappingArgs) -> ProfileEdits { - let via = m.via.as_deref().map(|raw| { - raw.split(',').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect::>() - }); + let via = (!m.via.is_empty()).then(|| m.via.clone()); ProfileEdits { local: m.local.clone(), remote: m.remote.clone(), @@ -33,12 +34,12 @@ pub fn edits_from_mapping(m: &MappingArgs) -> ProfileEdits { } /// `true` if `MappingArgs` carries no edits at all - used by `edit` to -/// reject a no-op invocation the same way vmic's commands do. +/// reject a no-op invocation. pub fn mapping_is_empty(m: &MappingArgs) -> bool { m.local.is_none() && m.remote.is_none() && m.dynamic.is_none() - && m.via.is_none() + && m.via.is_empty() && m.user.is_none() && m.identity.is_none() && m.port.is_none() diff --git a/src/commands/open.rs b/src/commands/open.rs index 22146de..ff426c4 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -56,9 +56,9 @@ fn open_one(name: &str, foreground: bool, once: bool) -> Result<()> { ui::info(&format!("'{name}' is already open (pid {pid}).")); return Ok(()); } - // A stale instance file left by a crash shouldn't linger through a - // fresh spawn - the lock is the real authority (spec §5.2), this just - // keeps `status` from showing ghost state mid-spawn. + // Clear a stale instance file left by a crash before spawning. The + // lock, not this file, is the authority on "already open" (spec + // §5.2) - this just keeps `status` from reading stale state mid-spawn. instance::delete(name)?; if foreground { @@ -100,12 +100,12 @@ fn spawn_detached(name: &str, once: bool) -> Result<()> { /// Blocks briefly for the detached supervisor to reach a conclusive state, /// so an immediate failure (bad auth, bind conflict, unresolvable host) is /// reported with a non-zero exit instead of `open` appearing to succeed -/// (spec §5.2). The instance file's *initial* write is always -/// `State::Reconnecting` (the first attempt hasn't concluded yet) - that's -/// indistinguishable from "already failed once, backing off" by state -/// alone, so this must wait specifically for `Up` or `Error`, not just -/// "anything other than Error", or it would report success before the -/// first connection attempt has even had a chance to run. +/// (spec §5.2). The instance file's initial write is always +/// `State::Reconnecting`, since the first attempt has not concluded yet; +/// that value is indistinguishable from "already failed once, backing +/// off". This function waits specifically for `Up` or `Error`, not merely +/// for a state other than `Error`, so it does not report success before +/// the first connection attempt has run. fn wait_for_confirmation(name: &str) -> Result<()> { let deadline = Instant::now() + CONFIRM_TIMEOUT; loop { diff --git a/src/commands/wipe.rs b/src/commands/wipe.rs index e647c6a..79ee09e 100644 --- a/src/commands/wipe.rs +++ b/src/commands/wipe.rs @@ -41,10 +41,10 @@ pub fn run(args: WipeArgs) -> Result<()> { Ok(()) } -/// Kills any supervisor process not backed by a tracked profile (e.g. -/// orphaned after a crash), matched by cmdline rather than tracked state - -/// same spirit as vmic's `wipe` (spec §5.8). SIGTERM to the whole process -/// group, same reasoning as `close_instance`'s force path. +/// Kills any supervisor process not backed by a tracked profile (e.g. one +/// orphaned after a crash), matched by cmdline rather than tracked state +/// (spec §5.8). Sends SIGTERM to each supervisor's process group so its +/// `ssh` child is included. fn kill_orphaned_supervisors() -> u32 { let mut killed = 0; let Ok(entries) = std::fs::read_dir("/proc") else { return 0 }; diff --git a/src/instance.rs b/src/instance.rs index d02b7a6..42ed25d 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -33,7 +33,7 @@ pub struct Instance { pub state: State, /// Anchor for "session uptime" (spec §5.5) - set once, when `open` starts. pub opened_at: i64, - /// Start of the *current* unbroken connection; resets each reconnect. + /// Start of the current unbroken connection; resets each reconnect. pub connected_at: Option, pub last_error: Option, pub reconnect_count: u32, @@ -77,8 +77,8 @@ pub fn log_path(name: &str) -> PathBuf { state_dir().join(format!("{name}.log")) } -/// Loads the instance file for `name`, if any. `None` means "closed" - the -/// absence of this file *is* the closed state (spec §2.2); there's no +/// Loads the instance file for `name`, if any. `None` means closed - the +/// absence of this file is the closed state (spec §2.2); there is no /// separate enum value for it. pub fn load(name: &str) -> Result> { let path = instance_path(name); @@ -109,9 +109,8 @@ pub fn process_alive(pid: i32) -> bool { unsafe { libc::kill(pid, 0) == 0 } } -/// True only if `pid` is a live process whose cmdline looks like our own -/// supervisor for `name` - insurance against a stale/reused pid, same -/// spirit as vmic's `pid_matches` check on `pw-loopback` processes. +/// True only if `pid` is a live process whose cmdline identifies it as the +/// supervisor for `name` - guards against a stale or reused pid. pub fn supervisor_alive(pid: i32, name: &str) -> bool { if !process_alive(pid) { return false; @@ -134,12 +133,10 @@ pub fn running_pid(name: &str) -> Result> { } } -/// Advisory `flock` held for the supervisor's entire lifetime (spec §2.3) - -/// releases automatically when the holding process exits or is killed, no -/// matter how abruptly, which is what makes it a reliable "is a supervisor -/// actually alive for this profile" primitive even across a crash: the OS -/// drops the lock the instant the fd closes, no stale-lock cleanup needed -/// the way a plain pidfile would require. +/// Advisory `flock` held for the supervisor's entire lifetime (spec §2.3). +/// The OS releases it the instant the holding process's file descriptors +/// close, including on a crash or SIGKILL, so it needs no stale-lock +/// cleanup and reliably answers "is a supervisor running for this profile." pub struct Lock { _file: File, } diff --git a/src/main.rs b/src/main.rs index da4a000..c3fb680 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,8 +16,8 @@ fn main() { let args: Vec = std::env::args().collect(); // Plain `porthole`, `-h`/`--help`, or `help` at the top level: show - // every subcommand's own flags inline instead of making the user drill - // into each one with its own `--help` - same convention as vmic. + // every subcommand's own flags inline instead of requiring a + // per-subcommand `--help`. if wants_top_level_help(&args) { print_full_help(); std::process::exit(if args.len() <= 1 { 2 } else { 0 }); @@ -65,11 +65,9 @@ fn wants_top_level_help(args: &[String]) -> bool { } } -/// Prints one screen of help: one short summary line per subcommand (name, -/// positional args, about), followed by an indented line per flag with its -/// own help text - so nothing requires drilling into a subcommand's own -/// `--help` just to see what it takes. Ported from vmic's `print_full_help` -/// so the two tools present identically. +/// Prints one screen of help: one summary line per subcommand (name, +/// positional args, description), followed by an indented line per flag +/// with its own help text. fn print_full_help() { let mut cmd = Cli::command(); cmd.build(); // resolve default value names etc. before introspecting @@ -149,8 +147,8 @@ fn positional_args(cmd: &clap::Command) -> String { } /// `(flag display, help text)` for each of `cmd`'s non-positional, non-help -/// args, e.g. `("-l/--local ", "Local forward: your machine -> -/// remote")`. The flag display is colored yellow. +/// args, e.g. `("-l/--local <[BIND:]PORT:HOST:PORT>", "Local forward: your +/// machine -> remote")`. The flag display is colored yellow. fn flag_rows(cmd: &clap::Command) -> Vec<(String, String)> { cmd.get_arguments() .filter(|a| !a.is_positional() && a.get_id().as_str() != "help") diff --git a/src/profile.rs b/src/profile.rs index 04da784..49fdc3c 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -237,10 +237,9 @@ impl Profile { } /// Splits `via` into the `-J` jump-chain value (comma-joined, all but - /// the last hop - `None` for a single-hop `via`) and the final `ssh` - /// connection target - see spec §3.1. `via` is validated non-empty by - /// every path that constructs a `Profile`, so the empty case here is - /// unreachable in practice. + /// the last hop; `None` for a single-hop `via`) and the final `ssh` + /// connection target - see spec §3.1. Every path that constructs a + /// `Profile` validates `via` as non-empty. pub fn ssh_target(&self) -> (Option, &str) { match self.via.split_last() { Some((target, jumps)) if !jumps.is_empty() => (Some(jumps.join(",")), target.as_str()), diff --git a/src/supervisor.rs b/src/supervisor.rs index c8d43bd..18a3815 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -19,8 +19,8 @@ const STABLE_THRESHOLD_SECS: i64 = 60; /// Consecutive unrecognized (not pattern-matched) failures before porthole /// gives up on an apparently-permanently-broken profile - spec §4.2. const MAX_UNRECOGNIZED_STREAK: u32 = 10; -/// How long `ssh` must stay alive before porthole calls it "connected" - -/// see `run_ssh_once`'s doc comment for why this heuristic is used at all. +/// How long `ssh` must stay alive before porthole treats it as connected; +/// see `run_ssh_once` for the heuristic this backs. const CONNECT_GRACE: Duration = Duration::from_secs(2); const POLL_INTERVAL: Duration = Duration::from_millis(200); const LOG_ROTATE_BYTES: u64 = 10 * 1024 * 1024; @@ -31,12 +31,12 @@ extern "C" fn handle_sigterm(_sig: libc::c_int) { SHUTDOWN.store(true, Ordering::SeqCst); } -/// Traps SIGTERM (and SIGINT, for `-f/--foreground`'s Ctrl-C - spec §5.2) -/// into a flag instead of the default terminate-immediately behavior, so -/// `close` is distinguished from a dropped `ssh` connection by *why* the -/// loop is unwinding, not by guessing from `ssh`'s exit status - which is -/// not a reliable signal either way. In foreground mode this function runs -/// in the same process the terminal sends Ctrl-C's SIGINT to, since +/// Traps SIGTERM and SIGINT into a flag instead of the default +/// terminate-immediately behavior. This is how `close` (SIGTERM) and +/// `-f/--foreground`'s Ctrl-C (SIGINT, spec §5.2) are distinguished from a +/// dropped `ssh` connection: by which signal arrived, not by inferring +/// intent from `ssh`'s exit status. In foreground mode this function runs +/// in the process the terminal sends Ctrl-C to directly, since /// `commands::open` calls `supervisor::run` inline rather than detaching. fn install_signal_handler() { unsafe { @@ -56,8 +56,9 @@ enum Outcome { Failed { class: Class, message: String }, } -/// Entry point for `porthole __supervise `. Runs until told to stop -/// (SIGTERM) or gives up per §4 - this *is* the supervisor process. +/// Entry point for `porthole __supervise `. This function is the +/// supervisor process: it runs until told to stop (SIGTERM/SIGINT) or +/// gives up per §4. pub fn run(name: &str) -> Result<()> { install_signal_handler(); @@ -141,10 +142,10 @@ fn sleep_or_shutdown(dur: Duration) -> bool { /// Spawns one `ssh` attempt and supervises it until it exits or shutdown is /// requested. Marks `inst` as `State::Up` once the process has survived -/// `CONNECT_GRACE` - `ssh` gives no more reliable "the forward is actually -/// bound" signal than that without parsing `-v` debug output, and a real -/// failure exits near-instantly under `ExitOnForwardFailure=yes` (§3.1), so -/// staying alive past the grace window is a reasonable proxy for connected. +/// `CONNECT_GRACE`. `ssh` does not report "the forward is bound" directly +/// without parsing `-v` debug output; a real failure exits near-instantly +/// under `ExitOnForwardFailure=yes` (§3.1), so staying alive past the grace +/// window is used as a proxy for connected. fn run_ssh_once(name: &str, profile: &Profile, inst: &mut Instance) -> Outcome { rotate_log_if_large(name); diff --git a/src/timefmt.rs b/src/timefmt.rs index c4bae72..2be2854 100644 --- a/src/timefmt.rs +++ b/src/timefmt.rs @@ -1,7 +1,7 @@ -//! Minimal UTC timestamp/duration formatting - no `chrono`/`time` dependency, -//! matching vmic's minimal-dependency footprint. Timestamps are stored as -//! Unix seconds (`i64`) everywhere in profile/instance state; this module -//! only turns them into text for `status`/`list` output. +//! Minimal UTC timestamp/duration formatting, without a `chrono`/`time` +//! dependency. Timestamps are stored as Unix seconds (`i64`) everywhere in +//! profile/instance state; this module only turns them into text for +//! `status`/`list` output. use std::time::{SystemTime, UNIX_EPOCH}; diff --git a/src/ui.rs b/src/ui.rs index 12cc1f1..3d86fb0 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -1,5 +1,4 @@ -//! Colorized status output (NO_COLOR-aware). Same conventions as vmic's -//! `ui.rs` - kept identical on purpose so the two tools feel like one family. +//! Colorized status output, honoring `NO_COLOR` and terminal detection. use std::io::IsTerminal; use std::sync::OnceLock;