fix(agent): probe package manager availability instead of hardcoding capabilities

devolutions-agent/src/broker/command_builder/dotnet.rs

@@ -120,7 +120,7 @@ fn dotnet_source(request: &PackageRequest) -> anyhow::Result<&str> {
     Ok(NUGET_ORG_V3_SOURCE)
 }
 
-fn trusted_dotnet_executable() -> String {
+pub(crate) fn trusted_dotnet_executable() -> String {
     let program_files = std::env::var("ProgramFiles")
         .ok()
         .filter(|path| trusted_program_files_path(path))

devolutions-agent/src/broker/executor/mod.rs

@@ -4,7 +4,7 @@
 
 use async_trait::async_trait;
 use chrono::{DateTime, Utc};
-use now_policy_api::{Elevation, Scope};
+use now_policy_api::{Elevation, ManagerName, Scope};
 use tracing::info;
 use win_api_wrappers::identity::sid::Sid;
 
@@ -57,6 +57,21 @@ pub struct ExecutionContext {
 
 pub type ProcessStartedCallback = std::sync::Arc<dyn Fn(DateTime<Utc>) + Send + Sync>;
 
+/// All package managers the broker knows how to drive.
+pub const BROKER_SUPPORTED_MANAGERS: [ManagerName; 11] = [
+    ManagerName::Winget,
+    ManagerName::Chocolatey,
+    ManagerName::Bun,
+    ManagerName::Cargo,
+    ManagerName::Dotnet,
+    ManagerName::Pip,
+    ManagerName::Npm,
+    ManagerName::PowerShell,
+    ManagerName::PowerShell7,
+    ManagerName::Scoop,
+    ManagerName::Vcpkg,
+];
+
 /// Trait for command execution strategies.
 #[async_trait]
 pub trait CommandExecutor: Send + Sync {
@@ -69,6 +84,16 @@ pub trait CommandExecutor: Send + Sync {
         ctx: &ExecutionContext,
         process_started: Option<ProcessStartedCallback>,
     ) -> anyhow::Result<ExecutionOutput>;
+
+    /// Probe which package managers are actually available for the target user.
+    ///
+    /// The default implementation reports no managers at all: availability must be
+    /// positively established by a platform executor, which overrides this to resolve
+    /// each manager executable using the same logic the execution path uses.
+    async fn probe_managers(&self, user_sid: &Sid) -> Vec<ManagerName> {
+        let _ = user_sid;
+        Vec::new()
+    }
 }
 
 /// Dry-run executor that only logs commands without running them.

devolutions-agent/src/broker/executor/windows/mod.rs

@@ -9,19 +9,22 @@ use std::path::{Path, PathBuf};
 use anyhow::{Context as _, bail};
 use async_trait::async_trait;
 use devolutions_agent_shared::temp_file::{BATCH_UTF8_PREAMBLE, POWERSHELL_UTF8_ENCODING_PREAMBLE, TmpFileGuard};
-use now_policy_api::{Elevation, Scope};
+use now_policy_api::{Elevation, ManagerName, Scope};
 use tracing::{debug, info, warn};
+use win_api_wrappers::identity::sid::Sid;
 use win_api_wrappers::process::Process;
-use win_api_wrappers::security::privilege::{self, ScopedPrivileges};
+use win_api_wrappers::security::privilege;
 use win_api_wrappers::token::Token;
 use win_api_wrappers::utils::WideString;
-use windows::Win32::Security::{TOKEN_ADJUST_PRIVILEGES, TOKEN_ALL_ACCESS, TOKEN_QUERY};
+use windows::Win32::Security::TOKEN_ALL_ACCESS;
 
-use super::{CommandExecutor, ExecutionContext, ExecutionOutput, ProcessStartedCallback};
+use super::{BROKER_SUPPORTED_MANAGERS, CommandExecutor, ExecutionContext, ExecutionOutput, ProcessStartedCallback};
 
+mod privileges;
 mod process;
 mod token;
 
+use privileges::SharedPrivileges;
 use process::create_process;
 use token::{detect_running_as_system, find_user_session, get_elevated_token};
 
@@ -90,6 +93,100 @@ impl CommandExecutor for WindowsExecutor {
         .await
         .context("blocking task panicked")?
     }
+
+    async fn probe_managers(&self, user_sid: &Sid) -> Vec<ManagerName> {
+        let is_system = self.is_system;
+        let user_sid = user_sid.clone();
+
+        // All Win32 calls are blocking — run in a blocking thread.
+        let probed = tokio::task::spawn_blocking(move || -> anyhow::Result<Vec<ManagerName>> {
+            let user_env = probe_user_environment(is_system, &user_sid)?;
+            Ok(BROKER_SUPPORTED_MANAGERS
+                .into_iter()
+                .filter(|manager| manager_is_available(*manager, &user_env))
+                .collect())
+        })
+        .await
+        .context("blocking task panicked")
+        .and_then(std::convert::identity);
+
+        match probed {
+            Ok(managers) => managers,
+            Err(error) => {
+                // Fail closed: the same session/environment lookup is required for execution,
+                // so a manager that cannot be verified cannot run either.
+                warn!(
+                    error = format!("{error:#}"),
+                    "Failed to probe package manager availability; advertising no managers"
+                );
+                Vec::new()
+            }
+        }
+    }
+}
+
+/// Load the environment block of the target user identified by `user_sid`.
+///
+/// In SYSTEM (service) mode the token comes from the user's active session, matching the
+/// token later used for execution. In user (development) mode the current process token is used.
+fn probe_user_environment(is_system: bool, user_sid: &Sid) -> anyhow::Result<HashMap<String, String>> {
+    let token = if is_system {
+        // WTSQueryUserToken (used by find_user_session) requires the SeTcb privilege.
+        let _privileges = SharedPrivileges::acquire(&[privilege::SE_TCB_NAME]).context("failed to enable SeTcb")?;
+
+        let (_session_id, user_token) =
+            find_user_session(user_sid).context("failed to find active session for user")?;
+        user_token
+    } else {
+        Process::current_process()
+            .token(TOKEN_ALL_ACCESS)
+            .context("failed to open current process token")?
+    };
+
+    win_api_wrappers::utils::environment_block(Some(&token), false).context("failed to load user environment block")
+}
+
+/// Check whether a package manager is usable for the target user, mirroring the executable
+/// resolution rules the execution path applies for that manager.
+fn manager_is_available(manager: ManagerName, user_env: &HashMap<String, String>) -> bool {
+    match manager {
+        ManagerName::Winget => resolve_winget_executable(user_env).is_ok(),
+        ManagerName::Chocolatey => default_chocolatey_install_dir()
+            .and_then(|root| resolve_trusted_chocolatey_executable(Some(user_env), &root))
+            .is_ok(),
+        ManagerName::Bun => resolve_bun_executable("bun", user_env).is_ok(),
+        ManagerName::Cargo => resolve_cargo_executable(user_env).is_ok(),
+        ManagerName::Dotnet => {
+            Path::new(&crate::broker::command_builder::dotnet::trusted_dotnet_executable()).is_file()
+        }
+        ManagerName::Pip => resolve_python_executable("python.exe", user_env).is_ok(),
+        // npm runs through the user PATH `npm` shim inside the trusted Windows PowerShell wrapper,
+        // so both the shim and the host must exist.
+        ManagerName::Npm => {
+            Path::new(&trusted_windows_powershell_executable()).is_file()
+                && path_contains_executable(user_env, &["npm.cmd", "npm.exe"])
+        }
+        ManagerName::PowerShell => Path::new(&trusted_windows_powershell_executable()).is_file(),
+        ManagerName::PowerShell7 => Path::new(&trusted_powershell7_executable()).is_file(),
+        // Scoop is driven through the user's `scoop.ps1` shim (resolved via `Get-Command
+        // -CommandType ExternalScript`) inside the trusted Windows PowerShell wrapper.
+        ManagerName::Scoop => {
+            Path::new(&trusted_windows_powershell_executable()).is_file()
+                && path_contains_executable(user_env, &["scoop.ps1"])
+        }
+        ManagerName::Vcpkg => resolve_vcpkg_executable(user_env).is_ok(),
+        _ => false,
+    }
+}
+
+/// Return true when any of `names` exists as a file in a directory listed in the environment PATH.
+fn path_contains_executable(env: &HashMap<String, String>, names: &[&str]) -> bool {
+    let path_var = env_value_ignore_case(env, "PATH").unwrap_or_default();
+    path_var
+        .split(';')
+        .map(str::trim)
+        .filter(|dir| !dir.is_empty())
+        .any(|dir| names.iter().any(|name| PathBuf::from(dir).join(name).is_file()))
 }
 
 /// Execute a command in the context of the target user's session (SYSTEM mode).
@@ -112,22 +209,16 @@ fn execute_as_system(
     );
 
     // Enable privileges required by CreateProcessAsUserW when running as SYSTEM.
-    // These are held by SYSTEM but not enabled by default.
-    let mut process_token = Process::current_process()
-        .token(TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY)
-        .context("failed to open process token for privilege adjustment")?;
-
-    debug!("Enabling SeTcb privilege");
-    let mut _priv_tcb =
-        ScopedPrivileges::enter(&mut process_token, &[privilege::SE_TCB_NAME]).context("failed to enable SeTcb")?;
-
-    debug!("Enabling SeAssignPrimaryToken privilege");
-    let mut _priv_primary = ScopedPrivileges::enter(_priv_tcb.token_mut(), &[privilege::SE_ASSIGNPRIMARYTOKEN_NAME])
-        .context("failed to enable SeAssignPrimaryToken")?;
-
-    debug!("Enabling SeIncreaseQuota privilege");
-    let _priv_quota = ScopedPrivileges::enter(_priv_primary.token_mut(), &[privilege::SE_INCREASE_QUOTA_NAME])
-        .context("failed to enable SeIncreaseQuota")?;
+    // These are held by SYSTEM but not enabled by default. The guard is reference-counted
+    // process-wide so concurrent requests (and availability probes) cannot disable a
+    // privilege out from under one another.
+    debug!("Enabling SeTcb, SeAssignPrimaryToken and SeIncreaseQuota privileges");
+    let _privileges = SharedPrivileges::acquire(&[
+        privilege::SE_TCB_NAME,
+        privilege::SE_ASSIGNPRIMARYTOKEN_NAME,
+        privilege::SE_INCREASE_QUOTA_NAME,
+    ])
+    .context("failed to enable privileges required for SYSTEM-mode execution")?;
 
     debug!("All privileges enabled, finding user session");
 

devolutions-agent/src/broker/executor/windows/privileges.rs

@@ -0,0 +1,138 @@
+//! Process-wide, reference-counted privilege enabling.
+//!
+//! `ScopedPrivileges` unconditionally disables privileges on drop, which is unsafe with
+//! concurrent broker requests: dropping one scope would disable a privilege another
+//! in-flight request still relies on (privileges are process-token state, not per-handle).
+//! [`SharedPrivileges`] keeps a per-privilege reference count and only enables a privilege
+//! on the first acquisition and disables it again when the last guard is dropped.
+
+use std::collections::HashMap;
+use std::sync::Mutex;
+
+use anyhow::Context as _;
+use tracing::error;
+use win_api_wrappers::process::Process;
+use win_api_wrappers::security::privilege;
+use win_api_wrappers::str::U16CStr;
+use win_api_wrappers::token::TokenPrivilegesAdjustment;
+use windows::Win32::Security::{TOKEN_ADJUST_PRIVILEGES, TOKEN_QUERY};
+
+static PRIVILEGE_REFCOUNTS: Mutex<Option<HashMap<String, usize>>> = Mutex::new(None);
+
+/// Guard keeping a set of process token privileges enabled.
+///
+/// Privileges are disabled on drop only when no other live guard still requires them.
+pub(super) struct SharedPrivileges {
+    names: Vec<&'static U16CStr>,
+}
+
+impl SharedPrivileges {
+    /// Enable `names` on the current process token, reference-counted across all guards.
+    pub(super) fn acquire(names: &[&'static U16CStr]) -> anyhow::Result<Self> {
+        let mut guard = PRIVILEGE_REFCOUNTS.lock().expect("privilege refcount lock poisoned");
+        let refcounts = guard.get_or_insert_with(HashMap::new);
+
+        let mut to_enable = Vec::new();
+        for name in names {
+            let count = refcounts.entry(name.to_string_lossy()).or_insert(0);
+            if *count == 0 {
+                to_enable.push(*name);
+            }
+            *count += 1;
+        }
+
+        if let Err(error) = adjust(&to_enable, true) {
+            // Roll back the reference counts taken above before reporting the failure.
+            for name in names {
+                if let Some(count) = refcounts.get_mut(&name.to_string_lossy()) {
+                    *count = count.saturating_sub(1);
+                }
+            }
+            return Err(error);
+        }
+
+        Ok(Self { names: names.to_vec() })
+    }
+}
+
+impl Drop for SharedPrivileges {
+    fn drop(&mut self) {
+        let mut guard = PRIVILEGE_REFCOUNTS.lock().expect("privilege refcount lock poisoned");
+        let Some(refcounts) = guard.as_mut() else {
+            return;
+        };
+
+        let mut to_disable = Vec::new();
+        for name in &self.names {
+            if let Some(count) = refcounts.get_mut(&name.to_string_lossy()) {
+                *count = count.saturating_sub(1);
+                if *count == 0 {
+                    to_disable.push(*name);
+                }
+            }
+        }
+
+        if let Err(error) = adjust(&to_disable, false) {
+            error!(error = format!("{error:#}"), "Failed to disable shared privileges");
+        }
+    }
+}
+
+/// Enable or disable `names` on the current process token.
+///
+/// Must be called with the refcount lock held so enable/disable operations are serialized.
+fn adjust(names: &[&U16CStr], enable: bool) -> anyhow::Result<()> {
+    if names.is_empty() {
+        return Ok(());
+    }
+
+    let mut luids = Vec::with_capacity(names.len());
+    for name in names {
+        luids.push(privilege::lookup_privilege_value(None, name).context("failed to look up privilege value")?);
+    }
+
+    let adjustment = if enable {
+        TokenPrivilegesAdjustment::Enable(luids)
+    } else {
+        TokenPrivilegesAdjustment::Disable(luids)
+    };
+
+    let mut token = Process::current_process()
+        .token(TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY)
+        .context("failed to open process token for privilege adjustment")?;
+    token
+        .adjust_privileges(&adjustment)
+        .context("failed to adjust privileges")
+}
+
+#[cfg(test)]
+mod tests {
+    #![allow(clippy::unwrap_used)]
+
+    use super::*;
+
+    #[test]
+    fn nested_guards_keep_privilege_enabled_until_last_drop() {
+        // SeChangeNotify is available to any token, making the test runnable without elevation.
+        let name = privilege::SE_CHANGE_NOTIFY_NAME;
+
+        let first = SharedPrivileges::acquire(&[name]).unwrap();
+        let second = SharedPrivileges::acquire(&[name]).unwrap();
+
+        drop(first);
+
+        {
+            let guard = PRIVILEGE_REFCOUNTS.lock().unwrap();
+            let refcounts = guard.as_ref().unwrap();
+            assert_eq!(refcounts.get(&name.to_string_lossy()), Some(&1));
+        }
+
+        drop(second);
+
+        {
+            let guard = PRIVILEGE_REFCOUNTS.lock().unwrap();
+            let refcounts = guard.as_ref().unwrap();
+            assert_eq!(refcounts.get(&name.to_string_lossy()), Some(&0));
+        }
+    }
+}