glasswing mythos fixes

src/VirtualClient/VirtualClient.Actions.UnitTests/HammerDB/HammerDBClientExecutorTests.cs

@@ -29,33 +29,50 @@ namespace VirtualClient.Actions
 
     [TestFixture]
     [Category("Unit")]
+    [NonParallelizable]
     public class HammerDBClientExecutorTests
     {
         private MockFixture fixture;
         private DependencyPath mockPackage;
         private string apiClientId;
         private ClientInstance clientInstance;
         private string mockPackagePath;
+        private string originalPassword;
 
-        [Test]
-        [TestCase(PlatformID.Unix, Architecture.X64)]
-        [TestCase(PlatformID.Win32NT, Architecture.X64)]
-        public async Task HammerDBClientExecutorRunsTheExpectedWorkloadCommand(PlatformID platform, Architecture architecture)
+        [SetUp]
+        public void SetPasswordEnvironmentVariable()
         {
-            this.SetupDefaultBehavior(platform, architecture);
+            this.originalPassword = Environment.GetEnvironmentVariable("PGPASSWORD");
+            Environment.SetEnvironmentVariable("PGPASSWORD", "password with 'quotes' and $symbols");
+        }
 
-            List<string> expectedCommands = new List<string>();
+        [TearDown]
+        public void RestorePasswordEnvironmentVariable()
+        {
+            Environment.SetEnvironmentVariable("PGPASSWORD", this.originalPassword);
+        }
 
-            if (platform == PlatformID.Unix)
-            {
-                expectedCommands.Add($"sudo python3 {this.fixture.PlatformSpecifics.Combine(this.mockPackagePath, "configure-workload-generator.py")} --workload tpcc --sqlServer postgresql --port 5432 --virtualUsers 1 --password [A-Fa-f0-9]+ --dbName hammerdbtest --hostIPAddress [0-9.]+ --warehouseCount 1 --duration 1");
-                expectedCommands.Add($"python3 {this.fixture.PlatformSpecifics.Combine(this.mockPackagePath, "run-workload.py")} --runTransactionsTCLFilePath runTransactions.tcl");
-            }
-            else
+        [Test]
+        [TestCase(PlatformID.Unix, Architecture.X64, "tpcc")]
+        [TestCase(PlatformID.Unix, Architecture.X64, "tpch")]
+        [TestCase(PlatformID.Win32NT, Architecture.X64, "tpcc")]
+        [TestCase(PlatformID.Win32NT, Architecture.X64, "tpch")]
+        public async Task HammerDBClientExecutorRunsTheExpectedWorkloadCommand(PlatformID platform, Architecture architecture, string workload)
+        {
+            this.SetupDefaultBehavior(platform, architecture);
+            this.fixture.Parameters[nameof(HammerDBClientExecutor.Workload)] = workload;
+            const string filePassword = "file-password-with-$symbols";
+            this.fixture.Parameters["PostgreSQLPasswordFile"] = "/secure/Password.json";
+            this.fixture.File.Setup(file => file.ReadAllTextAsync("/secure/Password.json", It.IsAny<CancellationToken>()))
+                .ReturnsAsync("{\"Password\":\"file-password-with-$symbols\"}");
+
+            string configurationCommand = platform == PlatformID.Unix ? "sudo --preserve-env=PGPASSWORD python3" : "python3";
+            string workloadArguments = workload == "tpcc" ? "--warehouseCount 1" : "--scaleFactor 10";
+            List<string> expectedCommands = new List<string>
             {
-                expectedCommands.Add($"python3 {this.fixture.PlatformSpecifics.Combine(this.mockPackagePath, "configure-workload-generator.py")} --workload tpcc --sqlServer postgresql --port 5432 --virtualUsers 1 --password [A-Fa-f0-9]+ --dbName hammerdbtest --hostIPAddress [0-9.]+ --warehouseCount 1 --duration 1");
-                expectedCommands.Add($"python3 {this.fixture.PlatformSpecifics.Combine(this.mockPackagePath, "run-workload.py")} --runTransactionsTCLFilePath runTransactions.tcl");
-            }
+                $"{configurationCommand} {this.fixture.PlatformSpecifics.Combine(this.mockPackagePath, "configure-workload-generator.py")} --workload {workload} --sqlServer postgresql --port 5432 --virtualUsers 1 --dbName hammerdbtest --hostIPAddress [0-9.]+ {workloadArguments} --duration 1",
+                $"python3 {this.fixture.PlatformSpecifics.Combine(this.mockPackagePath, "run-workload.py")} --runTransactionsTCLFilePath runTransactions.tcl"
+            };
 
             if (this.fixture.Platform == PlatformID.Win32NT)
             {
@@ -70,6 +87,9 @@ public async Task HammerDBClientExecutorRunsTheExpectedWorkloadCommand(PlatformI
             {
                 string expectedCommand = expectedCommands[commandNumber];
                 string executedCommand = $"{exe} {arguments}";
+                StringAssert.DoesNotContain("--password", arguments);
+                StringAssert.DoesNotContain(Environment.GetEnvironmentVariable("PGPASSWORD"), arguments);
+                StringAssert.DoesNotContain(filePassword, arguments);
                 bool executed = Regex.IsMatch(executedCommand,expectedCommands[commandNumber]);
                 Assert.IsTrue(executed);
                 commandNumber += 1;
@@ -86,13 +106,21 @@ public async Task HammerDBClientExecutorRunsTheExpectedWorkloadCommand(PlatformI
                     OnHasExited = () => true
                 };
 
+                process.OnStart = () =>
+                {
+                    Assert.AreEqual(filePassword, process.EnvironmentVariables["PGPASSWORD"]);
+                    return true;
+                };
+
                 return process;
             };
 
             using (TestHammerDBClientExecutor HammerDBExecutor = new TestHammerDBClientExecutor(this.fixture.Dependencies, this.fixture.Parameters))
             {
                 await HammerDBExecutor.ExecuteAsync(CancellationToken.None);
             }
+
+            Assert.AreEqual(expectedCommands.Count, commandNumber);
         }
         public void SetupDefaultBehavior(PlatformID platform, Architecture architecture)
         {
@@ -107,11 +135,11 @@ public void SetupDefaultBehavior(PlatformID platform, Architecture architecture)
             {
                 { nameof(HammerDBClientExecutor.Port), "5432"},
                 { nameof(HammerDBClientExecutor.DatabaseName), "hammerdbtest" },
-                { nameof(HammerDBClientExecutor.SuperUserPassword), "pass" },
                 { nameof(HammerDBClientExecutor.Workload), "tpcc" },
                 { nameof(HammerDBClientExecutor.SQLServer), "postgresql" },
                 { nameof(HammerDBClientExecutor.VirtualUserCount), "1"},
                 { nameof(HammerDBClientExecutor.WarehouseCount), "1"},
+                { nameof(HammerDBClientExecutor.ScaleFactor), "10"},
                 { nameof(HammerDBClientExecutor.Duration), "00:01:00" },
                 { "ServerIpAddress", "localhost"},
                 { nameof(HammerDBClientExecutor.PackageName), "hammerdb" },

src/VirtualClient/VirtualClient.Actions.UnitTests/HammerDB/HammerDBExecutorTests.cs

@@ -4,48 +4,40 @@
 namespace VirtualClient.Actions
 {
     using System;
-    using System.CodeDom.Compiler;
     using System.Collections.Generic;
     using System.Diagnostics;
     using System.IO;
-    using System.Linq;
-    using System.Net;
-    using System.Net.Http;
-    using System.Reflection;
+    using System.Runtime.InteropServices;
     using System.Threading;
     using System.Threading.Tasks;
-    using AutoFixture;
     using Microsoft.Extensions.DependencyInjection;
     using Moq;
-    using Newtonsoft.Json;
     using Newtonsoft.Json.Linq;
     using NUnit.Framework;
-    using Polly;
     using VirtualClient.Common;
     using VirtualClient.Common.Extensions;
     using VirtualClient.Common.Telemetry;
     using VirtualClient.Contracts;
-    using static VirtualClient.Actions.HammerDBExecutor;
 
     [TestFixture]
     [Category("Unit")]
+    [NonParallelizable]
     public class HammerDBExecutorTests
     {
         private MockFixture fixture;
         private DependencyPath mockPackage;
-        private string scriptPath;
-        private string mockPackagePath;
+        private string originalPassword;
 
         [SetUp]
         public void SetUpTests()
         {
+            this.originalPassword = Environment.GetEnvironmentVariable("PGPASSWORD");
+            Environment.SetEnvironmentVariable("PGPASSWORD", null);
             this.fixture = new MockFixture();
             this.fixture.Setup(PlatformID.Unix);
             this.mockPackage = new DependencyPath("hammerdb", this.fixture.PlatformSpecifics.GetPackagePath("hammerdb"));
             this.fixture.PackageManager.OnGetPackage().ReturnsAsync(this.mockPackage);
 
-            this.mockPackagePath = this.mockPackage.Path;
-            this.scriptPath = this.fixture.PlatformSpecifics.GetScriptPath("hammerdb");
             this.fixture.Layout = new EnvironmentLayout(new List<ClientInstance>
             {
                 new ClientInstance($"{Environment.MachineName}-Server", "1.2.3.4", "Server"),
@@ -56,8 +48,102 @@ public void SetUpTests()
             this.fixture.File.Setup(f => f.Exists(It.IsAny<string>())).Returns(true);
         }
 
+        [TearDown]
+        public void RestorePasswordEnvironment()
+        {
+            Environment.SetEnvironmentVariable("PGPASSWORD", this.originalPassword);
+        }
+
+        [TestCase(Architecture.X64)]
+        [TestCase(Architecture.Arm64)]
+        public async Task PostgreSQLPasswordIsReadFromTheArchitectureSpecificPackage(Architecture architecture)
+        {
+            this.fixture.Setup(PlatformID.Unix, architecture);
+            DependencyPath package = new DependencyPath("postgresql", this.fixture.GetPackagePath("postgresql"));
+            this.fixture.PackageManager.OnGetPackage().ReturnsAsync(package);
+            string platformPath = this.fixture.ToPlatformSpecificPath(package, PlatformID.Unix, architecture).Path;
+            this.fixture.Directory.Setup(directory => directory.Exists(platformPath)).Returns(true);
+            string passwordFile = this.fixture.PlatformSpecifics.Combine(platformPath, "Password.json");
+            this.fixture.File.Setup(file => file.Exists(passwordFile)).Returns(true);
+            this.fixture.File.Setup(file => file.ReadAllTextAsync(passwordFile, It.IsAny<CancellationToken>()))
+                .ReturnsAsync("{\"Password\":\"file password with 'quotes' and $symbols\"}");
+
+            using TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters);
+            Assert.AreEqual("file password with 'quotes' and $symbols", await executor.OnGetPostgreSQLPassword(CancellationToken.None));
+            Assert.IsNull(Environment.GetEnvironmentVariable("PGPASSWORD"));
+            this.fixture.File.Verify(file => file.ReadAllTextAsync(passwordFile, It.IsAny<CancellationToken>()), Times.Once);
+        }
+
+        [Test]
+        public async Task PostgreSQLPasswordExplicitFileTakesPrecedenceOverEnvironment()
+        {
+            Environment.SetEnvironmentVariable("PGPASSWORD", "environment-password");
+            const string passwordFile = "/secure/Password.json";
+            this.fixture.Parameters["PostgreSQLPasswordFile"] = passwordFile;
+            this.fixture.File.Setup(file => file.ReadAllTextAsync(passwordFile, It.IsAny<CancellationToken>()))
+                .ReturnsAsync("{\"Password\":\"file-password\"}");
+
+            using TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters);
+            Assert.AreEqual("file-password", await executor.OnGetPostgreSQLPassword(CancellationToken.None));
+            Assert.AreEqual("environment-password", Environment.GetEnvironmentVariable("PGPASSWORD"));
+        }
+
+        [Test]
+        public async Task PostgreSQLPasswordEnvironmentFallbackDoesNotReadAFile()
+        {
+            Environment.SetEnvironmentVariable("PGPASSWORD", "environment-password");
+
+            using TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters);
+            Assert.AreEqual("environment-password", await executor.OnGetPostgreSQLPassword(CancellationToken.None));
+            this.fixture.File.Verify(file => file.ReadAllTextAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never);
+        }
+
+        [TestCase("")]
+        [TestCase("{}")]
+        [TestCase("{\"Password\":\"\"}")]
+        [TestCase("{\"Password\":null}")]
+        [TestCase("{\"Password\":123}")]
+        [TestCase("{\"Password\":\"sensitive-sentinel\",")]
+        public void PostgreSQLPasswordRejectsInvalidFilesWithoutDisclosingTheirContents(string contents)
+        {
+            const string passwordFile = "/secure/Password.json";
+            this.fixture.Parameters["PostgreSQLPasswordFile"] = passwordFile;
+            this.fixture.File.Setup(file => file.ReadAllTextAsync(passwordFile, It.IsAny<CancellationToken>())).ReturnsAsync(contents);
+
+            using TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters);
+            DependencyException exception = Assert.ThrowsAsync<DependencyException>(() => executor.OnGetPostgreSQLPassword(CancellationToken.None));
+            Assert.AreEqual(ErrorReason.InvalidProfileDefinition, exception.Reason);
+            StringAssert.DoesNotContain("sensitive-sentinel", exception.ToString());
+        }
+
         [Test]
-        public void HammerDBPExecutorThrowsOnUnsupportedDistroAsync()
+        public void PostgreSQLPasswordRejectsMissingExplicitFileWithoutFallingBackToEnvironment()
+        {
+            Environment.SetEnvironmentVariable("PGPASSWORD", "environment-password");
+            this.fixture.Parameters["PostgreSQLPasswordFile"] = "/secure/missing.json";
+            this.fixture.File.Setup(file => file.Exists("/secure/missing.json")).Returns(false);
+
+            using TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters);
+            DependencyException exception = Assert.ThrowsAsync<DependencyException>(() => executor.OnGetPostgreSQLPassword(CancellationToken.None));
+            Assert.AreEqual(ErrorReason.InvalidProfileDefinition, exception.Reason);
+            StringAssert.DoesNotContain("environment-password", exception.ToString());
+        }
+
+        [Test]
+        public void PostgreSQLPasswordReadErrorsDoNotExposeUnderlyingExceptionDetails()
+        {
+            const string passwordFile = "/secure/Password.json";
+            this.fixture.Parameters["PostgreSQLPasswordFile"] = passwordFile;
+            this.fixture.File.Setup(file => file.ReadAllTextAsync(passwordFile, It.IsAny<CancellationToken>()))
+                .ThrowsAsync(new IOException("sensitive-sentinel"));
+
+            using TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters);
+            DependencyException exception = Assert.ThrowsAsync<DependencyException>(() => executor.OnGetPostgreSQLPassword(CancellationToken.None));
+            StringAssert.DoesNotContain("sensitive-sentinel", exception.ToString());
+        }
+
+        [Test]
+        public void HammerDBExecutorThrowsOnUnsupportedDistroAsync()
         {
             LinuxDistributionInfo mockInfo = new LinuxDistributionInfo()
             {
@@ -74,22 +160,64 @@ public void HammerDBPExecutorThrowsOnUnsupportedDistroAsync()
             }
         }
 
-        [Test]
-        public void SuperUserPasswordUsesHexEncodingWithNoSpecialCharacters()
+        [TestCase(0, "password with 'quotes' and $symbols")]
+        [TestCase(1, "password with 'quotes' and $symbols")]
+        [TestCase(0, "1")]
+        public async Task ProcessOutputObscuresCredentialInTelemetryFilesAndFailures(int exitCode, string password)
         {
-            using (TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters))
+            this.fixture.Parameters[nameof(HammerDBExecutor.LogToFile)] = true;
+            List<string> logFiles = new List<string>();
+            List<string> loggedOutput = new List<string>();
+            this.fixture.File.Setup(file => file.WriteAllTextAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
+                .Callback<string, string, CancellationToken>((path, content, token) =>
+                {
+                    if (path.EndsWith(".log", StringComparison.Ordinal))
+                    {
+                        logFiles.Add(content);
+                    }
+                })
+                .Returns(Task.CompletedTask);
+            this.fixture.Logger.OnLog = (level, eventInfo, state, error) =>
             {
-                string password = executor.SuperUserPassword;
+                if (eventInfo.Name.EndsWith(".ProcessDetails", StringComparison.Ordinal))
+                {
+                    JObject details = JObject.FromObject(((EventContext)state).Properties["process"]);
+                    loggedOutput.Add(details["standardOutput"].Value<string>());
+                    loggedOutput.Add(details["standardError"].Value<string>());
+                }
+            };
+
+            using InMemoryProcess process = new InMemoryProcess
+            {
+                StartInfo = new ProcessStartInfo { FileName = "hammerdbcli", Arguments = "auto runTransactions.tcl" },
+                ExitCode = exitCode,
+                OnHasExited = () => true
+            };
+            string output = $"Changed tpcc:pg_pass from postgres to {password} for PostgreSQL\nTransactions/sec: 120";
+            process.StandardOutput.Append(output);
+            process.StandardError.Append($"Connection details: {password}");
 
-                // Password must be hex-encoded (alphanumeric only) to avoid breaking
-                // HammerDB TCL diset command argument parsing.
-                Assert.IsTrue(System.Text.RegularExpressions.Regex.IsMatch(password, "^[A-Fa-f0-9]+$"),
-                    $"SuperUserPassword must be hex-encoded (alphanumeric only). Got: {password}");
+            using TestHammerDBExecutor executor = new TestHammerDBExecutor(this.fixture.Dependencies, this.fixture.Parameters);
+            await executor.OnLogPostgreSQLProcessDetails(process, password, EventContext.None, "HammerDB");
 
-                // Must not contain Base64 special characters that break TCL parsing
-                Assert.IsFalse(password.Contains("+"), "Password must not contain '+' (breaks TCL parsing)");
-                Assert.IsFalse(password.Contains("="), "Password must not contain '=' (breaks TCL parsing)");
-                Assert.IsFalse(password.Contains("/"), "Password must not contain '/' (breaks TCL parsing)");
+            Assert.AreEqual(1, logFiles.Count);
+            Assert.AreEqual(2, loggedOutput.Count);
+            Assert.AreEqual(output.Replace(password, "...", StringComparison.Ordinal), loggedOutput[0]);
+            Assert.AreEqual("Connection details: ...", loggedOutput[1]);
+            StringAssert.Contains(loggedOutput[0], logFiles[0]);
+            StringAssert.Contains(loggedOutput[1], logFiles[0]);
+            StringAssert.DoesNotContain($"to {password} for PostgreSQL", logFiles[0]);
+            StringAssert.DoesNotContain($"Connection details: {password}", logFiles[0]);
+
+            if (exitCode == 0)
+            {
+                Assert.AreEqual(output, process.StandardOutput.ToString(), "Metric input must remain unchanged.");
+            }
+            else
+            {
+                WorkloadException exception = Assert.Throws<WorkloadException>(() =>
+                    process.ThrowIfErrored<WorkloadException>(process.StandardError.ToString(), ErrorReason.WorkloadFailed));
+                StringAssert.DoesNotContain(password, exception.ToString());
             }
         }
 
@@ -100,7 +228,9 @@ public TestHammerDBExecutor(IServiceCollection dependencies, IDictionary<string,
             {
             }
 
-            public Func<EventContext, CancellationToken, Task> OnInitialize => base.InitializeAsync;
+            public Func<CancellationToken, Task<string>> OnGetPostgreSQLPassword => base.GetPostgreSQLPasswordAsync;
+
+            public Func<IProcessProxy, string, EventContext, string, Task> OnLogPostgreSQLProcessDetails => base.LogPostgreSQLProcessDetailsAsync;
         }
     }
 }