diff --git a/src/bin/dosh-client.rs b/src/bin/dosh-client.rs index 6582038..f022f85 100644 --- a/src/bin/dosh-client.rs +++ b/src/bin/dosh-client.rs @@ -37,7 +37,7 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; -use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream, UdpSocket, UnixStream}; use tokio::signal::unix::{SignalKind, signal}; @@ -1173,15 +1173,10 @@ fn select_session(requested: Option<&str>, _force_new: bool) -> String { if let Some(session) = requested { return session.to_string(); } - unique_session_name() -} - -fn unique_session_name() -> String { - let millis = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_millis(); - format!("term-{millis}-{}", std::process::id()) + // Implicit, ephemeral session: the server recognizes this name shape + // (`protocol::is_implicit_session_name`) and does NOT persist it across + // restarts, since you can never reattach to a random name. + protocol::generate_implicit_session_name() } fn ssh_bootstrap( diff --git a/src/bin/dosh-server.rs b/src/bin/dosh-server.rs index 4e392d1..65a3583 100644 --- a/src/bin/dosh-server.rs +++ b/src/bin/dosh-server.rs @@ -472,6 +472,20 @@ impl ServerState { Ok(()) } + /// Whether a session named `name` should be backed by a persistent holder. + /// + /// Persistence is only worthwhile for sessions a user can actually reattach + /// to by name: prewarmed sessions and explicitly-named ones (`--session`). + /// Implicit `term--` names (from `dosh host` with no session) + /// are ephemeral — persisting them just leaves unreattachable holders + /// lingering across restarts, so they use the in-process (dies-on-restart) + /// shell that reaps normally on disconnect. + fn should_persist_session(&self, name: &str) -> bool { + self.config.persist_sessions + && (self.config.prewarm_sessions.iter().any(|s| s == name) + || !protocol::is_implicit_session_name(name)) + } + /// Spawn (or, in the persistent case, spawn-and-adopt) a PTY for a session. /// /// With `persist_sessions` on, the shell is launched in a detached holder @@ -488,7 +502,7 @@ impl ServerState { ) -> Result<(PtyHandle, Option, bool)> { let cols = cols.max(1); let rows = rows.max(1); - if self.config.persist_sessions { + if self.should_persist_session(name) { match self.spawn_persistent_pty(name, cols, rows, env) { Ok(pair) => return Ok((pair.0, Some(pair.1), true)), Err(err) => { @@ -2911,6 +2925,33 @@ fn parse_env_pairs(raw: &[String]) -> Result> { mod tests { use super::*; + #[test] + fn persists_named_and_prewarmed_but_not_implicit_sessions() { + let (pty_tx, _rx) = mpsc::unbounded_channel(); + let config = ServerConfig { + persist_sessions: true, + prewarm_sessions: vec!["default".to_string()], + ..ServerConfig::default() + }; + let state = ServerState::new(config, [0u8; 32], pty_tx); + assert!(state.should_persist_session("default")); // prewarmed + assert!(state.should_persist_session("work")); // explicitly named + assert!(!state.should_persist_session("term-1781470634216-76685")); // implicit + } + + #[test] + fn persist_disabled_never_persists() { + let (pty_tx, _rx) = mpsc::unbounded_channel(); + let config = ServerConfig { + persist_sessions: false, + prewarm_sessions: vec!["default".to_string()], + ..ServerConfig::default() + }; + let state = ServerState::new(config, [0u8; 32], pty_tx); + assert!(!state.should_persist_session("default")); + assert!(!state.should_persist_session("work")); + } + #[test] fn native_auth_rate_limiter_throttles_per_ip_and_refills() { let mut limiter = NativeAuthRateLimiter::new(3); diff --git a/src/protocol.rs b/src/protocol.rs index e075d59..1c0bae1 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -3,6 +3,39 @@ use crate::crypto; use crate::native::{EnvVar, NativeAuthOk, NativeClientHello, NativeServerHello, NativeUserAuth}; use anyhow::{Context, Result, bail}; use serde::{Deserialize, Serialize}; +use std::time::{SystemTime, UNIX_EPOCH}; + +/// Generate a name for an implicit, ephemeral terminal session — the kind +/// created by `dosh host` with no `--session`. Single source of truth shared by +/// the client (which generates it) and the server (which recognizes it via +/// [`is_implicit_session_name`] to decide a session is NOT worth persisting). +pub fn generate_implicit_session_name() -> String { + let millis = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_millis(); + format!("term-{millis}-{}", std::process::id()) +} + +/// Whether `name` is a client-generated implicit session name +/// (`term--`). Such sessions are ephemeral: the user can never +/// reattach by name, so the server must not persist them across restarts. +/// Explicitly-named (`--session work`) and prewarmed sessions are not implicit. +pub fn is_implicit_session_name(name: &str) -> bool { + let Some(rest) = name.strip_prefix("term-") else { + return false; + }; + let mut parts = rest.split('-'); + match (parts.next(), parts.next(), parts.next()) { + (Some(millis), Some(pid), None) => { + !millis.is_empty() + && millis.bytes().all(|b| b.is_ascii_digit()) + && !pid.is_empty() + && pid.bytes().all(|b| b.is_ascii_digit()) + } + _ => false, + } +} pub const MAGIC: &[u8; 4] = b"DOSH"; // v3: added `ForwardingKind::Agent` (SSH-agent forwarding). The new variant rides @@ -496,3 +529,41 @@ impl ReplayWindow { } } } + +#[cfg(test)] +mod session_name_tests { + use super::{generate_implicit_session_name, is_implicit_session_name}; + + #[test] + fn generated_names_are_recognized_as_implicit() { + let name = generate_implicit_session_name(); + assert!(is_implicit_session_name(&name), "generated: {name}"); + } + + #[test] + fn explicit_and_prewarm_names_are_not_implicit() { + for name in [ + "default", + "work", + "logs", + "term", + "term-", + "term-abc-1", + "term-1-x", + "term-1", + "term-1-2-3", + "", + ] { + assert!( + !is_implicit_session_name(name), + "should not be implicit: {name:?}" + ); + } + } + + #[test] + fn implicit_shape_matches() { + assert!(is_implicit_session_name("term-1781470634216-76685")); + assert!(is_implicit_session_name("term-0-0")); + } +}