From de5710b9ad1eb77bc1c13766d05955744fba1ff2 Mon Sep 17 00:00:00 2001 From: Richard Markiewicz Date: Fri, 18 Sep 2026 09:31:04 -0400 Subject: [PATCH] fix(agent): start the session process only for remote sessions Co-Authored-By: Claude Opus 5 (1M context) --- devolutions-agent/src/session_manager.rs | 59 +++++++++++++++++++++--- devolutions-session/src/main.rs | 23 ++++++++- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/devolutions-agent/src/session_manager.rs b/devolutions-agent/src/session_manager.rs index 13ba0c1c9..28c461ca5 100644 --- a/devolutions-agent/src/session_manager.rs +++ b/devolutions-agent/src/session_manager.rs @@ -32,6 +32,12 @@ enum SessionKind { Remote, } +impl SessionKind { + fn is_remote(self) -> bool { + matches!(self, Self::Remote) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum SessionReadiness { WaitingForLogon, @@ -89,7 +95,6 @@ impl GatewaySession { } } - #[allow(dead_code)] fn kind(&self) -> SessionKind { self.kind } @@ -99,8 +104,13 @@ impl GatewaySession { &self.session } + /// Whether the Devolutions Session process should be started for this session. + /// + /// The process exists to host the DVC handler, which only a remote session can serve: a + /// console session has no RDP client to open the dynamic virtual channel against, so starting + /// the process there yields one that fails the open with `ERROR_FILE_NOT_FOUND` and exits. fn is_ready_to_start(&self, event: SessionReadinessEvent) -> bool { - self.readiness.is_satisfied_by(event) + self.kind().is_remote() && self.readiness.is_satisfied_by(event) } fn set_session_ready(&mut self) { @@ -196,10 +206,12 @@ impl Task for SessionManager { AgentServiceEvent::SessionConnect(id) => { info!(%id, "Session connected"); let mut ctx = ctx.write().await; + // The session is registered so that a later upgrade to a remote + // session is tracked, but the session process is only ever started for + // remote sessions (initiated via RDP), because the DVC handler it hosts + // is only reachable from one. `GatewaySession::is_ready_to_start` + // enforces that. ctx.register_session(&id, SessionKind::Console, SessionReadiness::WaitingForLogon); - // We only start the session process for remote sessions (initiated - // via RDP), as session process with DVC handler is only needed for remote - // sessions. } AgentServiceEvent::SessionDisconnect(id) => { info!(%id, "Session disconnected"); @@ -389,7 +401,11 @@ fn session_app_path() -> Utf8PathBuf { #[cfg(test)] mod tests { - use super::{SessionReadiness, SessionReadinessEvent}; + use super::{GatewaySession, Session, SessionKind, SessionReadiness, SessionReadinessEvent}; + + fn make_session(kind: SessionKind, readiness: SessionReadiness) -> GatewaySession { + GatewaySession::new(Session::new(1), kind, readiness) + } #[test] fn new_user_session_waits_for_logon() { @@ -414,4 +430,35 @@ mod tests { assert!(SessionReadiness::WaitingForLogonOrUnlock.is_satisfied_by(SessionReadinessEvent::Logon)); assert!(SessionReadiness::WaitingForLogonOrUnlock.is_satisfied_by(SessionReadinessEvent::Unlock)); } + + #[test] + fn console_session_never_starts_the_session_process() { + for readiness in [ + SessionReadiness::WaitingForLogon, + SessionReadiness::WaitingForUnlock, + SessionReadiness::WaitingForLogonOrUnlock, + ] { + let session = make_session(SessionKind::Console, readiness); + + assert!(!session.is_ready_to_start(SessionReadinessEvent::Logon)); + assert!(!session.is_ready_to_start(SessionReadinessEvent::Unlock)); + } + } + + #[test] + fn remote_session_starts_on_its_readiness_event() { + let session = make_session(SessionKind::Remote, SessionReadiness::WaitingForLogon); + assert!(session.is_ready_to_start(SessionReadinessEvent::Logon)); + + let session = make_session(SessionKind::Remote, SessionReadiness::WaitingForUnlock); + assert!(session.is_ready_to_start(SessionReadinessEvent::Unlock)); + } + + #[test] + fn ready_remote_session_is_not_started_twice() { + let session = make_session(SessionKind::Remote, SessionReadiness::Ready); + + assert!(!session.is_ready_to_start(SessionReadinessEvent::Logon)); + assert!(!session.is_ready_to_start(SessionReadinessEvent::Unlock)); + } } diff --git a/devolutions-session/src/main.rs b/devolutions-session/src/main.rs index 97fea4c8c..cfde821c9 100644 --- a/devolutions-session/src/main.rs +++ b/devolutions-session/src/main.rs @@ -33,7 +33,10 @@ fn main() -> anyhow::Result<()> { let _logger_guard = init_log(&conf); - info!("Starting Devolutions Session"); + match session_id_from_args() { + Some(session_id) => info!(%session_id, "Starting Devolutions Session"), + None => info!("Starting Devolutions Session"), + } let (runtime, shutdown_handle, join_handle) = start()?; @@ -53,6 +56,24 @@ fn main() -> anyhow::Result<()> { Ok(()) } +/// Returns the session ID the Devolutions Agent passed via `--session`, if any. +/// +/// The agent starts this process in a specific Windows session and names that session on the +/// command line. Logging it is what makes a session log answerable on its own: the DVC can only +/// be opened against the session the process actually runs in, so a failure to open it is only +/// interpretable once that session is known. +fn session_id_from_args() -> Option { + let mut args = std::env::args().skip(1); + + while let Some(arg) = args.next() { + if arg == "--session" { + return args.next(); + } + } + + None +} + pub fn start() -> anyhow::Result<(Runtime, ShutdownHandle, JoinHandle<()>)> { let runtime = tokio::runtime::Builder::new_multi_thread() .enable_all()