Expand environment variables in custom install arguments and location

src/UniGetUI.Core.Tools.Tests/ToolsTests.cs

@@ -1,4 +1,5 @@
 using System.Diagnostics;
+using System.Runtime.InteropServices;
 using UniGetUI.Core.Language;
 using UniGetUI.PackageEngine.Enums;
 
@@ -50,6 +51,73 @@ public void TestStaticallyLoadedLanguages()
             );
         }
 
+        [Fact]
+        public void EscapeCommandLineArgument_WrapsSimplePathInQuotes()
+        {
+            Assert.Equal("\"C:\\dev\\contoso\"", CoreTools.EscapeCommandLineArgument(@"C:\dev\contoso"));
+            Assert.Equal("\"C:\\Program Files\\App\"", CoreTools.EscapeCommandLineArgument(@"C:\Program Files\App"));
+        }
+
+        [Fact]
+        public void EscapeCommandLineArgument_EscapesEmbeddedQuoteToPreventInjection()
+        {
+            Assert.Equal("\"C:\\x\\\" --evil\"", CoreTools.EscapeCommandLineArgument("C:\\x\" --evil"));
+        }
+
+        [Fact]
+        public void EscapeCommandLineArgument_DoublesTrailingBackslashes()
+        {
+            Assert.Equal("\"C:\\App\\\\\"", CoreTools.EscapeCommandLineArgument(@"C:\App\"));
+        }
+
+        [Theory]
+        [InlineData(@"C:\dev\contoso")]
+        [InlineData(@"C:\Program Files\App")]
+        [InlineData(@"C:\App\")]
+        [InlineData("C:\\x\" --disable-hash-check")]
+        [InlineData(@"C:\weird path\with spaces\and\")]
+        [InlineData("plain")]
+        [InlineData("")]
+        public void EscapeCommandLineArgument_RoundTripsThroughWindowsParser(string argument)
+        {
+            if (!OperatingSystem.IsWindows())
+                return;
+
+            string escaped = CoreTools.EscapeCommandLineArgument(argument);
+            string[] parsed = SplitWindowsCommandLine("app.exe " + escaped);
+
+            Assert.Equal(2, parsed.Length);
+            Assert.Equal(argument, parsed[1]);
+        }
+
+        [DllImport("shell32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
+        private static extern IntPtr CommandLineToArgvW(string lpCmdLine, out int pNumArgs);
+
+        [DllImport("kernel32.dll", SetLastError = true)]
+        private static extern IntPtr LocalFree(IntPtr hMem);
+
+        private static string[] SplitWindowsCommandLine(string commandLine)
+        {
+            IntPtr argv = CommandLineToArgvW(commandLine, out int argc);
+            if (argv == IntPtr.Zero)
+                throw new System.ComponentModel.Win32Exception(Marshal.GetLastWin32Error());
+
+            try
+            {
+                string[] result = new string[argc];
+                for (int i = 0; i < argc; i++)
+                {
+                    IntPtr entry = Marshal.ReadIntPtr(argv, i * IntPtr.Size);
+                    result[i] = Marshal.PtrToStringUni(entry) ?? "";
+                }
+                return result;
+            }
+            finally
+            {
+                LocalFree(argv);
+            }
+        }
+
         [Fact]
         public async Task TestWhichFunctionForExistingFile()
         {

src/UniGetUI.Core.Tools/Tools.cs

@@ -1012,6 +1012,41 @@ public static string GetCurrentLocale()
         public static string MakeValidFileName(string name) =>
             string.Concat(name.Where(x => !_illegalPathChars.Contains(x)));
 
+        public static string EscapeCommandLineArgument(string argument)
+        {
+            StringBuilder builder = new();
+            builder.Append('"');
+            int i = 0;
+            while (i < argument.Length)
+            {
+                int backslashes = 0;
+                while (i < argument.Length && argument[i] == '\\')
+                {
+                    i++;
+                    backslashes++;
+                }
+
+                if (i == argument.Length)
+                {
+                    builder.Append('\\', backslashes * 2);
+                }
+                else if (argument[i] == '"')
+                {
+                    builder.Append('\\', backslashes * 2 + 1);
+                    builder.Append('"');
+                    i++;
+                }
+                else
+                {
+                    builder.Append('\\', backslashes);
+                    builder.Append(argument[i]);
+                    i++;
+                }
+            }
+            builder.Append('"');
+            return builder.ToString();
+        }
+
         // Safely wait for a task that may throw an exception we don't care about
         public static async void FinalizeDangerousTask(Task t)
         {

src/UniGetUI.PackageEngine.Managers.Cargo/Helpers/CargoPkgOperationHelper.cs

@@ -1,3 +1,4 @@
+using UniGetUI.Core.Tools;
 using UniGetUI.PackageEngine.Classes.Manager.BaseProviders;
 using UniGetUI.PackageEngine.Enums;
 using UniGetUI.PackageEngine.Interfaces;
@@ -52,7 +53,7 @@ OperationType operation
                     parameters.Add("--skip-signatures");
 
                 if (options.CustomInstallLocation != "")
-                    parameters.AddRange(["--install-path", options.CustomInstallLocation]);
+                    parameters.AddRange(["--install-path", CoreTools.EscapeCommandLineArgument(options.CustomInstallLocation)]);
             }
         }
 

src/UniGetUI.PackageEngine.Managers.Dotnet/Helpers/DotNetPkgOperationHelper.cs

@@ -1,3 +1,4 @@
+using UniGetUI.Core.Tools;
 using UniGetUI.PackageEngine.Classes.Manager.BaseProviders;
 using UniGetUI.PackageEngine.Enums;
 using UniGetUI.PackageEngine.Interfaces;
@@ -30,7 +31,7 @@ OperationType operation
         ];
 
         if (options.CustomInstallLocation != "")
-            parameters.AddRange(["--tool-path", "\"" + options.CustomInstallLocation + "\""]);
+            parameters.AddRange(["--tool-path", CoreTools.EscapeCommandLineArgument(options.CustomInstallLocation)]);
 
         if (
             package.OverridenOptions.Scope is PackageScope.Global

src/UniGetUI.PackageEngine.Managers.WinGet/Helpers/WinGetPkgOperationHelper.cs

@@ -115,14 +115,14 @@ operation is OperationType.Uninstall
                 var effectiveLocation = GetEffectiveUpdateLocation(package, options);
                 if (effectiveLocation is not null)
                 {
-                    parameters.AddRange(["--location", $"\"{effectiveLocation}\""]);
+                    parameters.AddRange(["--location", CoreTools.EscapeCommandLineArgument(effectiveLocation)]);
                 }
             }
         }
         else if (operation is OperationType.Install)
         {
             if (options.CustomInstallLocation != "")
-                parameters.AddRange(["--location", $"\"{options.CustomInstallLocation}\""]);
+                parameters.AddRange(["--location", CoreTools.EscapeCommandLineArgument(options.CustomInstallLocation)]);
         }
 
         if (operation is not OperationType.Uninstall)

src/UniGetUI.PackageEngine.PackageManagerClasses/Packages/Classes/InstallOptionsFactory.cs

@@ -220,42 +220,13 @@ private static InstallOptions _loadFromDisk(string key)
 
         private static InstallOptions EnsureSecureOptions(InstallOptions options)
         {
+            options.CustomInstallLocation = _expandEnvironmentVariables(options.CustomInstallLocation);
+
             if (SecureSettings.Get(SecureSettings.K.AllowCLIArguments))
             {
-                // If CLI arguments are allowed, sanitize them
-                for (int i = 0; i < options.CustomParameters_Install.Count; i++)
-                {
-                    options.CustomParameters_Install[i] = options
-                        .CustomParameters_Install[i]
-                        .Replace("&", "")
-                        .Replace("|", "")
-                        .Replace(";", "")
-                        .Replace("<", "")
-                        .Replace(">", "")
-                        .Replace("\n", "");
-                }
-                for (int i = 0; i < options.CustomParameters_Update.Count; i++)
-                {
-                    options.CustomParameters_Update[i] = options
-                        .CustomParameters_Update[i]
-                        .Replace("&", "")
-                        .Replace("|", "")
-                        .Replace(";", "")
-                        .Replace("<", "")
-                        .Replace(">", "")
-                        .Replace("\n", "");
-                }
-                for (int i = 0; i < options.CustomParameters_Uninstall.Count; i++)
-                {
-                    options.CustomParameters_Uninstall[i] = options
-                        .CustomParameters_Uninstall[i]
-                        .Replace("&", "")
-                        .Replace("|", "")
-                        .Replace(";", "")
-                        .Replace("<", "")
-                        .Replace(">", "")
-                        .Replace("\n", "");
-                }
+                _expandAndSanitizeCliArguments(options.CustomParameters_Install);
+                _expandAndSanitizeCliArguments(options.CustomParameters_Update);
+                _expandAndSanitizeCliArguments(options.CustomParameters_Uninstall);
             }
             else
             {
@@ -313,5 +284,36 @@ private static InstallOptions EnsureSecureOptions(InstallOptions options)
 
             return options;
         }
+
+        private static void _expandAndSanitizeCliArguments(List<string> parameters)
+        {
+            for (int i = 0; i < parameters.Count; i++)
+            {
+                parameters[i] = _expandEnvironmentVariables(parameters[i])
+                    .Replace("&", "")
+                    .Replace("|", "")
+                    .Replace(";", "")
+                    .Replace("<", "")
+                    .Replace(">", "")
+                    .Replace("\n", "");
+            }
+        }
+
+        private static string _expandEnvironmentVariables(string value)
+        {
+            if (string.IsNullOrEmpty(value) || !value.Contains('%'))
+                return value;
+
+            try
+            {
+                return Environment.ExpandEnvironmentVariables(value);
+            }
+            catch (Exception ex)
+            {
+                Logger.Warn($"Could not expand environment variables in \"{value}\"");
+                Logger.Warn(ex);
+                return value;
+            }
+        }
     }
 }

src/UniGetUI.PackageEngine.Tests/InstallOptionsFactoryTests.cs

@@ -155,6 +155,68 @@ public void LoadApplicable_SanitizesCustomParametersWhenCliArgumentsAreAllowed()
         Assert.Equal(["--keepdrop"], resolved.CustomParameters_Install);
     }
 
+    [Fact]
+    public void LoadApplicable_ExpandsEnvironmentVariablesInCustomParametersAndLocation()
+    {
+        var varName = $"UNIGETUI_TEST_{Guid.NewGuid():N}";
+        Environment.SetEnvironmentVariable(varName, @"C:\Expanded");
+        try
+        {
+            var manager = new PackageManagerBuilder().WithName($"Manager{Guid.NewGuid():N}").Build();
+            var package = new PackageBuilder().WithManager(manager).WithId($"Pkg{Guid.NewGuid():N}").Build();
+            var packageOptions = new InstallOptions
+            {
+                OverridesNextLevelOpts = true,
+                CustomInstallLocation = $"%{varName}%\\app",
+                CustomParameters_Install = [$"--location=%{varName}%\\app"],
+                CustomParameters_Update = [$"--location=%{varName}%"],
+                CustomParameters_Uninstall = ["--purge"],
+            };
+
+            SecureSettings.ApplyForUser(Environment.UserName, SecureSettings.ResolveKey(SecureSettings.K.AllowCLIArguments), true);
+            InstallOptionsFactory.SaveForPackage(packageOptions, package);
+
+            var resolved = InstallOptionsFactory.LoadApplicable(package);
+
+            Assert.Equal(@"C:\Expanded\app", resolved.CustomInstallLocation);
+            Assert.Equal([@"--location=C:\Expanded\app"], resolved.CustomParameters_Install);
+            Assert.Equal([@"--location=C:\Expanded"], resolved.CustomParameters_Update);
+            Assert.Equal(["--purge"], resolved.CustomParameters_Uninstall);
+        }
+        finally
+        {
+            Environment.SetEnvironmentVariable(varName, null);
+        }
+    }
+
+    [Fact]
+    public void LoadApplicable_SanitizesMetacharactersIntroducedByEnvironmentVariableExpansion()
+    {
+        var varName = $"UNIGETUI_TEST_{Guid.NewGuid():N}";
+        Environment.SetEnvironmentVariable(varName, "safe & rm -rf");
+        try
+        {
+            var manager = new PackageManagerBuilder().WithName($"Manager{Guid.NewGuid():N}").Build();
+            var package = new PackageBuilder().WithManager(manager).WithId($"Pkg{Guid.NewGuid():N}").Build();
+            var packageOptions = new InstallOptions
+            {
+                OverridesNextLevelOpts = true,
+                CustomParameters_Install = [$"--flag=%{varName}%"],
+            };
+
+            SecureSettings.ApplyForUser(Environment.UserName, SecureSettings.ResolveKey(SecureSettings.K.AllowCLIArguments), true);
+            InstallOptionsFactory.SaveForPackage(packageOptions, package);
+
+            var resolved = InstallOptionsFactory.LoadApplicable(package);
+
+            Assert.Equal(["--flag=safe  rm -rf"], resolved.CustomParameters_Install);
+        }
+        finally
+        {
+            Environment.SetEnvironmentVariable(varName, null);
+        }
+    }
+
     [Fact]
     public void SaveAndLoadForPackage_RoundTripsPersistedOptions()
     {

src/UniGetUI.PackageEngine.Tests/WinGetManagerTests.cs

@@ -2,6 +2,7 @@
 using Devolutions.Pinget.Core;
 using UniGetUI.Core.Data;
 using UniGetUI.Core.SettingsEngine;
+using UniGetUI.Core.Tools;
 using UniGetUI.PackageEngine.Classes.Manager;
 using UniGetUI.PackageEngine.Enums;
 using UniGetUI.PackageEngine.Interfaces;
@@ -883,6 +884,32 @@ public void WinGetUpdateEmitsExplicitLocationEvenWhenForceSettingIsOff()
         Assert.Contains("\"D:\\dev\\contoso\"", parameters);
     }
 
+    [Fact]
+    public void WinGetInstallEscapesQuotesInCustomLocationToPreventArgumentInjection()
+    {
+        var manager = new WinGet();
+        SetCliToolKind(manager, WinGetCliToolKind.SystemWinGet);
+        var package = new PackageBuilder()
+            .WithManager(manager)
+            .WithId("Contoso.QuotedLocation")
+            .WithVersion("1.0.0")
+            .Build();
+        var options = new InstallOptions
+        {
+            CustomInstallLocation = "C:\\apps\" --evil-injected-switch",
+        };
+
+        var parameters = manager.OperationHelper.GetParameters(package, options, OperationType.Install).ToList();
+
+        int locationIndex = parameters.IndexOf("--location");
+        Assert.True(locationIndex >= 0);
+        Assert.Equal(
+            CoreTools.EscapeCommandLineArgument("C:\\apps\" --evil-injected-switch"),
+            parameters[locationIndex + 1]
+        );
+        Assert.DoesNotContain("--evil-injected-switch", parameters);
+    }
+
     [Fact]
     public void WinGetUpdateOmitsInheritedLocationWhenForceSettingIsOff()
     {