From 14af0a7f1f80903c2edb69645d9861355f6daefa Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Wed, 29 Jul 2026 18:30:56 +0300 Subject: [PATCH] fix(agent): enforce client identity for user-scope execution in dev mode The broker Windows executor's development (non-SYSTEM) mode ran commands under the broker process token regardless of which user connected to the pipe, so a user-scope install requested by a different user would land in the broker user's profile. Reject execution when the pipe client SID does not match the broker process user SID. Also clarify the SYSTEM-mode error when the target user has no active logon session, since user-scope and interactive operations require one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/broker/executor/windows/mod.rs | 54 +++++++++++++++++-- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/devolutions-agent/src/broker/executor/windows/mod.rs b/devolutions-agent/src/broker/executor/windows/mod.rs index a143b06f5..85bbed997 100644 --- a/devolutions-agent/src/broker/executor/windows/mod.rs +++ b/devolutions-agent/src/broker/executor/windows/mod.rs @@ -124,8 +124,10 @@ fn execute_as_system( debug!("All privileges enabled, finding user session"); - let (session_id, user_token) = - find_user_session(&ctx.user_sid).context("failed to find active session for user")?; + let (session_id, user_token) = find_user_session(&ctx.user_sid).context( + "failed to find an active logon session for the target user; \ + user-scope and interactive operations require the user to be logged on", + )?; info!( effective_user = %ctx.effective_user, @@ -170,6 +172,11 @@ fn execute_as_system( /// /// Opens the current process token and uses the same `create_process_as_user` /// code path as SYSTEM mode, ensuring consistent behavior (environment, desktop, flags). +/// +/// The client identity must match the broker process user: the process token defines +/// the target profile (`%LOCALAPPDATA%`, HKCU) and desktop session, so executing on +/// behalf of a different pipe client would put user-scope installs into the wrong +/// profile and interactive UIs on the wrong desktop. fn execute_as_current_user( ctx: &ExecutionContext, process_started: Option, @@ -183,6 +190,20 @@ fn execute_as_current_user( .token(TOKEN_ALL_ACCESS) .context("failed to open current process token")?; + let process_user_sid = token + .sid_and_attributes() + .context("failed to query current process token user SID")? + .sid; + if process_user_sid != ctx.user_sid { + bail!( + "pipe client user SID '{}' does not match broker process user SID '{}'; \ + user-scope execution in development mode is only supported when the client \ + and the broker run as the same user", + ctx.user_sid, + process_user_sid, + ); + } + let session_id = token.session_id().context("failed to query token session ID")?; let output = run_plan(&token, ctx, session_id, process_started)?; @@ -1116,8 +1137,9 @@ mod tests { use win_api_wrappers::identity::sid::Sid; use super::{ - POWERSHELL_UTF8_ENCODING_PREAMBLE, prepare_chocolatey_script_in_with_default_install_root, - prepare_main_command_in, prepare_shell_command_in, reject_unsupported_vcpkg_elevation, + POWERSHELL_UTF8_ENCODING_PREAMBLE, execute_as_current_user, + prepare_chocolatey_script_in_with_default_install_root, prepare_main_command_in, prepare_shell_command_in, + reject_unsupported_vcpkg_elevation, }; use crate::broker::executor::ExecutionContext; @@ -1624,6 +1646,30 @@ mod tests { assert!(error.to_string().contains("machine-scope")); } + #[test] + fn dev_mode_rejects_client_user_different_from_broker_user() { + let ctx = ExecutionContext { + kill_processes: Vec::new(), + pre_command: None, + command: vec![ + r"C:\Windows\System32\cmd.exe".to_owned(), + "/C".to_owned(), + "exit".to_owned(), + ], + post_command: None, + effective_user: "DOMAIN\\other".to_owned(), + // The Everyone (World) SID never matches the test process user SID. + user_sid: Sid::from_well_known(windows::Win32::Security::WinWorldSid, None) + .expect("well-known Everyone SID"), + elevation: Elevation::Standard, + scope: Some(Scope::User), + capture_output: false, + }; + + let error = execute_as_current_user(&ctx, None).expect_err("mismatched client SID should fail"); + assert!(error.to_string().contains("does not match broker process user SID")); + } + #[test] fn pip_command_resolves_python_without_shell_wrapper() { let temp_dir = tempfile::tempdir().expect("create temp dir");