winapps-org / winapps

The winapps main project, forked from https://github.com/Fmstrat/winapps/
Other
998 stars 45 forks source link

Enhance VM Detection and Improve FreeRDP Command Handling #121

Closed KernelGhost closed 2 months ago

KernelGhost commented 3 months ago
  1. Improved detection of running VMs.
    • Replaced virsh list with virsh list --state-running --name to directly filter for running VMs by name.
    • Updated grep command to grep -q '^RDPWindows$' for precise matching of the VM name using a regular expression.
  2. Convert FreeRDP command to array format for improved readability and straightforward modification.
  3. Extract VM IPv4 address using a regular expression.
CLAassistant commented 3 months ago

CLA assistant check
All committers have signed the CLA.

LDprg commented 3 months ago

Great PR. However I am not sure about the 2 point, because I don't see how/why this is better.

KernelGhost commented 3 months ago

@LDprg It is recommended to use arrays for command construction, particularly when dealing with potentially complex or dynamically generated commands. In this context, command construction using arrays provides two main advantages:

  1. Arrays automatically handle quoting of arguments, ensuring that spaces and special characters in arguments are properly preserved. I was previously experiencing authentication failures when using a username that contained spaces prior to this adjustment.
  2. Arrays handle empty variables gracefully by skipping them, preventing the formation of double spaces. This was previously an (albeit small) issue when using variable interpolation directly (e.g. $MULTI_FLAG).

Although there are additional benefits related to preventing injection vulnerabilities, these are less relevant in this particular scenario.

For improved readability and clarity, the section of concern can be rewritten as follows:

COMMAND=(
    "${FREERDP_COMMAND}"                 # FreeRDP Command/Executable
    "/d:${RDP_DOMAIN}"                   # Remote Desktop Connection Domain
    "/u:${RDP_USER}"                     # Username
    "/p:${RDP_PASS}"                     # Password
    "/scale:${RDP_SCALE}"                # Remote Desktop Session Scaling Factor
    "+dynamic-resolution"                # Enable Dynamic Resolution Adjustment
    "+auto-reconnect"                    # Enable Automatic Reconnection on Disconnect
    "+home-drive"                        # Map User Home Drive
    "/wm-class:\"Microsoft Windows\""    # Window Manager
    "/v:${RDP_IP}"                       # Virtual Machine IP Address
)