(#3503, #3513) Handle arguments file decryption failures

src/chocolatey/infrastructure.app/services/NugetService.cs

@@ -189,6 +189,8 @@ it is possible that incomplete package lists are returned from a command
                 this.Log().Debug(ChocolateyLoggers.Verbose, () => "--- Start of List ---");
             }
 
+            var decryptionFailures = new List<ChocolateyPackageInformation>();
+
             foreach (var pkg in NugetList.GetPackages(config, _nugetLogger, _fileSystem))
             {
                 var package = pkg; // for lamda access
@@ -217,9 +219,24 @@ it is possible that incomplete package lists are returned from a command
                         }
                     }
 
-                    if (!string.IsNullOrWhiteSpace(packageInfo.Arguments))
+                    // The main scenario where we desire the decrypted arguments during a list sequence is to display them when verbose output is selected.
+                    // This is done by default on the `info` command, and by request on the `list` command. As such, we are going to validate it's that scenario
+                    // to avoid needlessly decrypting the arguments file.
+                    var shouldDecryptArguments = (
+                            config.CommandName.Equals("info", StringComparison.OrdinalIgnoreCase) ||
+                            config.CommandName.Equals("list", StringComparison.OrdinalIgnoreCase)
+                        ) &&
+                        config.Verbose &&
+                        !string.IsNullOrWhiteSpace(packageInfo.Arguments);
+
+                    if (shouldDecryptArguments)
                     {
-                        var decryptedArguments = ArgumentsUtility.DecryptPackageArgumentsFile(_fileSystem, packageInfo.Package.Id, packageInfo.Package.Version.ToNormalizedStringChecked());
+                        var decryptedArguments = ArgumentsUtility.DecryptPackageArgumentsFile(_fileSystem, packageInfo.Package.Id, packageInfo.Package.Version.ToNormalizedStringChecked()).ToList();
+
+                        if (decryptedArguments.Count <= 0)
+                        {
+                            decryptionFailures.Add(packageInfo);
+                        }
 
                         packageArgumentsUnencrypted = "\n Remembered Package Arguments: \n  {0}".FormatWith(string.Join(Environment.NewLine + "  ", decryptedArguments));
                     }
@@ -336,6 +353,23 @@ Package url{6}
                     this.Log().Warn(logType, "Over {0:N0} packages, or package versions, was found per source, but there may be more packages available that were filtered out. Please refine your search, or specify a page number to retrieve more results.".FormatWith(NugetList.LastPackageLimitUsed * 0.9));
                 }
             }
+
+            if (decryptionFailures.Count > 0)
+            {
+                var failedPackages = string.Join(", ", decryptionFailures.Select(f => "{0} - {1}".FormatWith(f.Package.Id, f.Package.Version)));
+                var failureMessage = "There were some failures decrypting package arguments.";
+                var failedPackagesMessage = "Failed packages: {0}".FormatWith(failedPackages);
+                if (config.RegularOutput)
+                {
+                    this.Log().Warn(failureMessage);
+                    this.Log().Warn(failedPackagesMessage);
+                }
+                else
+                {
+                    this.Log().Debug(failureMessage);
+                    this.Log().Debug(failedPackagesMessage);
+                }
+            }
         }
 
         public void PackDryRun(ChocolateyConfiguration config)
@@ -1915,38 +1949,19 @@ protected virtual ChocolateyConfiguration SetConfigFromRememberedArguments(Choco
                 return config;
             }
 
-            var packageArgumentsUnencrypted = packageInfo.Arguments.ContainsSafe(" --") && packageInfo.Arguments.ToStringSafe().Length > 4 ? packageInfo.Arguments : NugetEncryptionUtility.DecryptString(packageInfo.Arguments);
+            var packageArguments = ArgumentsUtility.DecryptPackageArgumentsFile(
+                _fileSystem, 
+                packageInfo.Package.Id,
+                packageInfo.Package.Version.ToNormalizedStringChecked(),
+                redactSensitiveArguments: false,
+                throwOnFailure: true).ToList();
+            var packageArgumentsUnencrypted = string.Join(" ", packageArguments);
 
-            var sensitiveArgs = true;
             if (!ArgumentsUtility.SensitiveArgumentsProvided(packageArgumentsUnencrypted))
             {
-                sensitiveArgs = false;
                 this.Log().Debug(ChocolateyLoggers.Verbose, "{0} - Adding remembered arguments for upgrade: {1}".FormatWith(packageInfo.Package.Id, packageArgumentsUnencrypted.EscapeCurlyBraces()));
             }
 
-            var packageArgumentsSplit = packageArgumentsUnencrypted.Split(new[] { " --" }, StringSplitOptions.RemoveEmptyEntries);
-            var packageArguments = new List<string>();
-            foreach (var packageArgument in packageArgumentsSplit.OrEmpty())
-            {
-                var packageArgumentSplit = packageArgument.Split(new[] { '=' }, 2, StringSplitOptions.RemoveEmptyEntries);
-                var optionName = packageArgumentSplit[0].ToStringSafe();
-                var optionValue = string.Empty;
-                if (packageArgumentSplit.Length == 2)
-                {
-                    optionValue = packageArgumentSplit[1].ToStringSafe().UnquoteSafe();
-                    if (optionValue.StartsWith("'"))
-                    {
-                        optionValue.UnquoteSafe();
-                    }
-                }
-
-                if (sensitiveArgs)
-                {
-                    this.Log().Debug(ChocolateyLoggers.Verbose, "{0} - Adding '{1}' to upgrade arguments. Values not shown due to detected sensitive arguments".FormatWith(packageInfo.Package.Id, optionName.EscapeCurlyBraces()));
-                }
-                packageArguments.Add("--{0}{1}".FormatWith(optionName, string.IsNullOrWhiteSpace(optionValue) ? string.Empty : "=" + optionValue));
-            }
-
             var originalConfig = config.DeepCopy();
             // this changes config globally
             ConfigurationOptions.OptionSet.Parse(packageArguments);

src/chocolatey/infrastructure.app/utility/ArgumentsUtility.cs

@@ -54,6 +54,16 @@ public static bool SensitiveArgumentsProvided(string commandArguments)
         }
 
         public static IEnumerable<string> DecryptPackageArgumentsFile(IFileSystem fileSystem, string id, string version)
+        {
+            return DecryptPackageArgumentsFile(fileSystem, id, version, redactSensitiveArguments: true, throwOnFailure: false);
+        }
+
+        public static IEnumerable<string> DecryptPackageArgumentsFile(
+            IFileSystem fileSystem,
+            string id,
+            string version,
+            bool redactSensitiveArguments,
+            bool throwOnFailure)
         {
             var argumentsPath = fileSystem.CombinePaths(ApplicationParameters.InstallLocation, ".chocolatey", "{0}.{1}".FormatWith(id, version));
             var argumentsFile = fileSystem.CombinePaths(argumentsPath, ".arguments");
@@ -79,11 +89,30 @@ public static IEnumerable<string> DecryptPackageArgumentsFile(IFileSystem fileSy
                 yield break;
             }
 
-            // The following code is borrowed from the Chocolatey codebase, should
-            // be extracted to a separate location in choco executable so we can re-use it.
-            var packageArgumentsUnencrypted = arguments.Contains(" --") && arguments.ToStringSafe().Length > 4
-                ? arguments
-                : NugetEncryptionUtility.DecryptString(arguments);
+            string packageArgumentsUnencrypted = string.Empty;
+
+            try
+            {
+                packageArgumentsUnencrypted = arguments.Contains(" --") && arguments.ToStringSafe().Length > 4
+                    ? arguments
+                    : NugetEncryptionUtility.DecryptString(arguments);
+
+            }
+            catch (Exception ex)
+            {
+                var firstMessage = "There was an error attempting to decrypt the contents of the .arguments file for version '{0}' of package '{1}'.  See log file for more information.".FormatWith(version, id);
+                var secondMessage = "We failed to decrypt '{0}'. Error from decryption:{1}  '{2}'".FormatWith(argumentsFile, Environment.NewLine, ex.Message.Trim());
+
+                if (throwOnFailure)
+                {
+                    "chocolatey".Log().Error(firstMessage);
+                    "chocolatey".Log().Error(secondMessage);
+                    throw;
+                }
+
+                "chocolatey".Log().Debug(firstMessage);
+                "chocolatey".Log().Debug(secondMessage);
+            }
 
             // Lets do a global check first to see if there are any sensitive arguments
             // before we filter out the values used later.
@@ -102,7 +131,7 @@ public static IEnumerable<string> DecryptPackageArgumentsFile(IFileSystem fileSy
                 var optionName = packageArgumentSplit[0].ToStringSafe();
                 var optionValue = string.Empty;
 
-                if (packageArgumentSplit.Length == 2 && isSensitiveArgument)
+                if (packageArgumentSplit.Length == 2 && isSensitiveArgument && redactSensitiveArguments)
                 {
                     optionValue = "[REDACTED ARGUMENT]";
                 }

tests/pester-tests/features/ArgumentsDecryption.Tests.ps1

@@ -0,0 +1,118 @@
+Describe 'Ensuring that corrupted .arguments file responds correctly for <CommandDescription> <Command>' -ForEach @(
+    @{
+        Command = 'install'
+        CommandDescription = 'default'
+        ExpectedExitCode = 0
+        Parameters = @('installpackage')
+    }
+    # It seems that upgrade and uninstall exit with -1 when the beforemodify fails.
+    # As such, these tests will expect those exit codes for success.
+    @{
+        Command = 'upgrade'
+        CommandDescription = 'default'
+        ExpectedExitCode = -1
+        Parameters = @('upgradepackage')
+    }
+    @{
+        Command = 'upgrade'
+        CommandDescription = 'remember'
+        ExpectedExitCode = 1
+        Parameters = @('upgradepackage')
+    }
+    @{
+        Command = 'download'
+        CommandDescription = 'default'
+        ExpectedExitCode = 0
+        Parameters = @('upgradepackage')
+    }
+    @{
+        Command = 'list'
+        CommandDescription = 'verbose'
+        ExpectedExitCode = 0
+        Parameters = @('--verbose')
+    }
+    @{
+        Command = 'list'
+        CommandDescription = 'default'
+        ExpectedExitCode = 0
+        Parameters = @()
+    }
+    @{
+        Command = 'info'
+        CommandDescription = 'verbose'
+        ExpectedExitCode = 0
+        Parameters = @('upgradepackage', '--local-only')
+    }
+    # It seems that upgrade and uninstall exit with -1 when the beforemodify fails.
+    # As such, these tests will expect those exit codes for success.
+    @{
+        Command = 'uninstall'
+        CommandDescription = 'default'
+        ExpectedExitCode = -1
+        Parameters = @('upgradepackage')
+    }
+    @{
+        Command = 'pin'
+        CommandDescription = 'default'
+        ExpectedExitCode = 0
+        Parameters = @('list')
+    }
+    @{
+        Command = 'pin'
+        CommandDescription = 'default'
+        ExpectedExitCode = 0
+        Parameters = @('add','-n=upgradepackage')
+    }
+) -Tag ArgumentsFileDecryption {
+    BeforeDiscovery {
+        $HasLicensedExtension = Test-PackageIsEqualOrHigher -PackageName 'chocolatey.extension' -Version '6.0.0'
+    }
+
+    BeforeAll {
+        Initialize-ChocolateyTestInstall
+    }
+
+    # Skip the download command if chocolatey.extension is not installed.
+    Context 'Command (<Command>) failure scenario (<ErrorType>)' -Skip:($Command -eq 'download' -and -not $HasLicensedExtension) -ForEach @(
+        @{
+            ErrorType = 'Base64 invalid'
+            DecryptionError = 'The input is not a valid Base-64 string as it contains a non-base 64 character, more than two padding characters, or an illegal character among the padding characters.'
+            # `!` is an invalid Base64 character: https://en.wikipedia.org/wiki/Base64#Base64_table_from_RFC_4648
+            FileContents = 'InvalidBase64!'
+        }
+        @{
+            ErrorType = 'Invalid decryption'
+            DecryptionError = 'Key not valid for use in specified state.'
+            # The contents of this was taken from a throw away VM. As such, DPAPI will not be able to decrypt it, and will error.
+            FileContents = 'AQAAANCMnd8BFdERjHoAwE/Cl+sBAAAAn1/taDnOFUqGb17fBymxHQQAAAACAAAAAAAQZgAAAAEAACAAAAAU8gmqznJYKdkuj8bgk8sgg6Le3sbGoGkZOV3YtRFfwwAAAAAOgAAAAAIAACAAAAD1I9LYxrEhx9m71eF3VqyAike+XJTePhDAcrOilAFjQlAAAAA8lfiMR5Ns/AntLdVR3eBQSduCnipRCbdu/er/+YABMTzJDMGqnXuIsKwWoNIhrB14Yit4jVPipt3a/Nx18xx+YsnUewI4P6GlDL5do1y8mkAAAABMxvyPgCtN36BwAOXvJghIh9Hs8jUZOJtQIlWci8BnJkBmaaoSZ6pTGULk4TbFXMf/FK1NPo2mPM0YVL8QgJyK'
+        }
+    ) {
+        BeforeAll {
+            Restore-ChocolateyInstallSnapshot
+
+            if ($CommandDescription -eq 'remember') {
+                Enable-ChocolateyFeature -Name useRememberedArgumentsForUpgrades
+            }
+
+            Invoke-Choco install upgradepackage --version 1.0.0
+            $argumentsFile = Join-Path $env:ChocolateyInstall ".chocolatey/upgradepackage.1.0.0/.arguments"
+            $FileContents | Set-Content -Path $argumentsFile -Encoding utf8 -Force
+
+            $Output = Invoke-Choco $Command @Parameters --debug
+        }
+
+        AfterAll {
+            Remove-ChocolateyInstallSnapshot
+        }
+
+        It 'Exits correctly (<ExpectedExitCode>)' {
+            $Output.ExitCode | Should -Be $ExpectedExitCode -Because $Output.String
+        }
+
+        It 'Outputs expected messages' {
+            $shouldContain = -not ($CommandDescription -eq 'verbose' -or $CommandDescription -eq 'remember')
+            $Output.Lines | Should -Not:$shouldContain -Contain "We failed to decrypt '$($env:ChocolateyInstall)\.chocolatey\upgradepackage.1.0.0\.arguments'. Error from decryption:" -Because $Output.String
+            $Output.Lines | Should -Not:$shouldContain -Contain "'$DecryptionError'" -Because $Output.String
+        }
+    }
+}