Merge pull request #3 from Sire/claude/review-project-practices-011CULPt6CJZr5NwAcwscQoM

Sire · web-flow · commit aca7f1fcee45 · 2025-10-22T12:58:28.000+02:00
Upgrades and new features
diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml
@@ -23,7 +23,7 @@ jobs:
       - name: Set up .NET
         uses: actions/setup-dotnet@v4
         with:
-          dotnet-version: '6.0.x'
+          dotnet-version: '8.0.x'
 
       - name: Restore dependencies
         run: dotnet restore
diff --git a/.gitignore b/.gitignore
@@ -361,4 +361,10 @@ MigrationBackup/
 
 # Fody - auto-generated XML schema
 FodyWeavers.xsd
+
+
+# Custom
 /Properties/launchSettings.json
+
+# Private todo list
+TODO.md
diff --git a/Commands/PingCommand.cs b/Commands/PingCommand.cs
@@ -1,8 +1,8 @@
 using System;
-using System.Data.SqlClient;
+using Microsoft.Data.SqlClient;
 using System.Reflection;
 using System.Text;
-using System.Threading;
+using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Spectre.Console;
@@ -19,62 +19,96 @@ public class PingCommand : AsyncCommand<ConsoleSettings>
         public override async Task<int> ExecuteAsync(CommandContext context, ConsoleSettings settings)
         {
             if (settings.SQLCommand == null)
-                settings.SQLCommand = $@"SELECT @@SERVERNAME AS ""Server"", name as ""Database"", state_desc AS ""State"", replica_id AS ""Replica"" FROM sys.databases WHERE name = '{settings.Database}' ";
+                settings.SQLCommand = @"SELECT @@SERVERNAME AS ""Server"", name as ""Database"", state_desc AS ""State"", replica_id AS ""Replica"" FROM sys.databases WHERE name = @DatabaseName";
 
-            //Logger.LogInformation("Connection string: {Mandatory}", connectionString);
-            //Logger.LogInformation("SQL Command: {Optional}", settings.SQLCommand);
-            //Logger.LogInformation("CommandOptionFlag: {CommandOptionFlag}", settings.CommandOptionFlag);
-            //Logger.LogInformation("CommandOptionValue: {CommandOptionValue}", settings.CommandOptionValue);
+            // Apply secure credential handling
+            ApplySecureCredentials(settings);
 
             var connString = GetConnectionString(settings);
 
-            //Logger.LogInformation("");
-            AnsiConsole.MarkupLine($"ConnectionString: [teal]{connString.EscapeMarkup()}[/]");
+            AnsiConsole.MarkupLine($"ConnectionString: [teal]{RedactConnectionString(connString).EscapeMarkup()}[/]");
             AnsiConsole.MarkupLine($"SQL Query       : [teal]{settings.SQLCommand.EscapeMarkup()}[/]");
 
+            // Helpful warning if using IP
+            if (LooksLikeIp(settings.Server)
+                && (settings.TrustServerCertificate != true)
+                && string.IsNullOrWhiteSpace(settings.HostNameInCertificate))
+            {
+                AnsiConsole.MarkupLine("[yellow]Hint:[/] You're connecting by IP. TLS certificate name validation usually fails with IPs unless the cert has the IP in SAN. Use a DNS name, set [teal]--hostname-in-certificate[/], or [teal]--trust-server-certificate true[/] for dev.");
+            }
+
             bool running = true;
 
             while (running)
             {
-
                 int sec = settings.Wait;
-                AnsiConsole.Status()
+                await AnsiConsole.Status()
                     .AutoRefresh(true)
-                    .Spinner(Spinner.Known.Dots) // https://jsfiddle.net/sindresorhus/2eLtsbey/embedded/result/
+                    .Spinner(Spinner.Known.Dots)
                     .SpinnerStyle(Style.Parse("green bold"))
-                    .Start("Please wait...", ctx =>
+                    .StartAsync("Please wait...", async ctx =>
                     {
-                        // Simulate some work
                         ctx.Status($"Trying to connect to server [teal]{settings.Server.EscapeMarkup()}[/]...");
-                        CallDatabase(connString, settings);
+                        await CallDatabaseAsync(connString, settings);
 
-                        // Update the status and spinner
                         ctx.Status($"Waiting [teal]{sec}[/] seconds...");
 
                         if (settings.NonStop)
-                            Thread.Sleep(TimeSpan.FromSeconds(sec));
+                            await Task.Delay(TimeSpan.FromSeconds(sec));
                         else
                             running = false;
                     });
             }
 
-            //Console.WriteLine("\nDone. Press enter.");
-            //Console.ReadLine();
-
             return await Task.FromResult(0);
         }
 
         // Validate as part of the command. This is a good way of validating options if you require any injected services.
         public override ValidationResult Validate(CommandContext context, ConsoleSettings settings)
         {
-            //if (settings.Wait < 1)
-            //    return ValidationResult.Error("...");
             return ValidationResult.Success();
         }
 
+        private static void ApplySecureCredentials(ConsoleSettings settings)
+        {
+            // Check for credentials from environment variables first
+            var envUsername = Environment.GetEnvironmentVariable("SQLPING_USERNAME");
+            var envPassword = Environment.GetEnvironmentVariable("SQLPING_PASSWORD");
+
+            bool passwordProvidedViaCommandLine = !string.IsNullOrEmpty(settings.Password);
+
+            // Use environment variables if command line values are not provided
+            if (string.IsNullOrEmpty(settings.Username) && !string.IsNullOrEmpty(envUsername))
+            {
+                settings.Username = envUsername;
+                AnsiConsole.MarkupLine("[yellow]Using username from SQLPING_USERNAME environment variable[/]");
+            }
+
+            if (string.IsNullOrEmpty(settings.Password) && !string.IsNullOrEmpty(envPassword))
+            {
+                settings.Password = envPassword;
+                AnsiConsole.MarkupLine("[yellow]Using password from SQLPING_PASSWORD environment variable[/]");
+            }
+
+            // If username is provided but password is not, prompt for password
+            if (!string.IsNullOrEmpty(settings.Username) && string.IsNullOrEmpty(settings.Password))
+            {
+                settings.Password = AnsiConsole.Prompt(
+                    new TextPrompt<string>("[yellow]Password:[/]")
+                        .PromptStyle("red")
+                        .Secret());
+            }
+
+            // Warn if password was provided via command line (security risk)
+            if (passwordProvidedViaCommandLine)
+            {
+                AnsiConsole.MarkupLine("[yellow]WARNING: Password provided via command line is visible in process list and shell history.[/]");
+                AnsiConsole.MarkupLine("[yellow]Consider using SQLPING_PASSWORD environment variable or interactive prompt instead.[/]");
+            }
+        }
+
         private static string GetConnectionString(ConsoleSettings settings) {
             SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder();
-            //builder.ConnectionString = settings.ConnectionString;
             builder.DataSource = settings.Server;
             if (!string.IsNullOrEmpty(settings.Username)) {
                 builder.UserID = settings.Username;
@@ -94,44 +128,155 @@ private static string GetConnectionString(ConsoleSettings settings) {
             builder.WorkstationID = Environment.MachineName;
             builder.ApplicationName = Assembly.GetExecutingAssembly().FullName;
 
+            // TLS related options
+            if (settings.TrustServerCertificate.HasValue)
+                builder.TrustServerCertificate = settings.TrustServerCertificate.Value;
+
+            if (!string.IsNullOrWhiteSpace(settings.Encrypt))
+            {
+                var encValue = settings.Encrypt.Trim().ToLowerInvariant();
+                if (encValue is "true" or "false" or "strict")
+                {
+                    builder["Encrypt"] = settings.Encrypt;
+                }
+                else
+                {
+                    AnsiConsole.MarkupLine("[yellow]Invalid --encrypt value. Use true|false|strict. Ignoring.[/]");
+                }
+            }
+
+            if (!string.IsNullOrWhiteSpace(settings.HostNameInCertificate))
+            {
+                builder["HostNameInCertificate"] = settings.HostNameInCertificate;
+            }
+
+            if (settings.NoTransparentNetworkIPResolution)
+            {
+                builder["TransparentNetworkIPResolution"] = "false";
+            }
+
             string connString = builder.ConnectionString;
             return connString;
         }
 
-        private void CallDatabase(string connString, ConsoleSettings settings)
+        private static string RedactConnectionString(string connectionString)
+        {
+            try
+            {
+                SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectionString);
+
+                // Redact password if present
+                if (!string.IsNullOrEmpty(builder.Password))
+                {
+                    builder.Password = "***REDACTED***";
+                }
+
+                return builder.ConnectionString;
+            }
+            catch
+            {
+                // If parsing fails, return a generic redacted message
+                return "***CONNECTION STRING REDACTED***";
+            }
+        }
+
+        private async Task CallDatabaseAsync(string connString, ConsoleSettings settings)
         {
             AnsiConsole.MarkupLine($"[teal]{DateTime.Now.ToLocalTime()}[/] Connecting to {settings.Server.EscapeMarkup()}... ");
 
             try
             {
-  
-                using (SqlConnection connection = new SqlConnection(connString))
+
+                await using (SqlConnection connection = new SqlConnection(connString))
                 {
-                    connection.Open();
+                    await connection.OpenAsync();
+
+                    // Ensure we are in the requested database even if Initial Catalog was not applied for any reason
+                    if (!string.IsNullOrWhiteSpace(settings.Database))
+                    {
+                        try
+                        {
+                            connection.ChangeDatabase(settings.Database);
+                        }
+                        catch (Exception dbEx)
+                        {
+                            AnsiConsole.MarkupLine($"[yellow]Warning:[/] Failed to change database to '[teal]{settings.Database.EscapeMarkup()}[/]': {dbEx.Message.EscapeMarkup()}");
+                        }
+                    }
+
                     var sb = new StringBuilder();
-                    using (SqlCommand command = new SqlCommand(settings.SQLCommand, connection))
+                    await using (SqlCommand command = new SqlCommand(settings.SQLCommand!, connection))
                     {
-                        using (SqlDataReader reader = command.ExecuteReader())
+                        // Add parameter if the query contains @DatabaseName placeholder
+                        if (settings.SQLCommand != null && settings.SQLCommand.Contains("@DatabaseName"))
+                        {
+                            command.Parameters.AddWithValue("@DatabaseName", settings.Database);
+                        }
+
+                        await using (SqlDataReader reader = await command.ExecuteReaderAsync())
                         {
-                            while (reader.Read())
+                            while (await reader.ReadAsync())
                             {
                                 for (int i = 0; i < reader.FieldCount; i++)
                                     if (reader.GetValue(i) != DBNull.Value)
                                         sb.Append($"{reader.GetName(i)}: {Convert.ToString(reader.GetValue(i))}  ");
-                                //sb.AppendLine();
                             }
                         }
                     }
-                    AnsiConsole.MarkupLine($"  [green]SUCCESS[/] {sb.ToString().EscapeMarkup()}");
+
+                    // Try to show connection encryption info, but ignore permission errors
+                    string info;
+                    string currentDbName = connection.Database;
+                    try
+                    {
+                        await using var infoCmd = new SqlCommand(
+                            "SELECT encrypt_option, net_transport FROM sys.dm_exec_connections WHERE session_id = @@SPID;", connection);
+                        await using var infoReader = await infoCmd.ExecuteReaderAsync();
+                        if (await infoReader.ReadAsync())
+                        {
+                            var encryptOption = Convert.ToString(infoReader["encrypt_option"]);
+                            var transport = Convert.ToString(infoReader["net_transport"]);
+                            info = $" (db={currentDbName}, encrypt_option={encryptOption}, net_transport={transport})";
+                        }
+                        else
+                        {
+                            info = $" (db={currentDbName})";
+                        }
+                    }
+                    catch (SqlException)
+                    {
+                        info = $" (db={currentDbName}, connection details unavailable: requires VIEW SERVER STATE)";
+                    }
+
+                    AnsiConsole.MarkupLine($"  [green]SUCCESS[/] {sb.ToString().EscapeMarkup()}{info}");
                 }
             }
             catch (SqlException ex)
             {
                 AnsiConsole.MarkupLine($"  [red]ERROR: {ex.Message.EscapeMarkup()}[/] ");
+
+                if (ex.Message.IndexOf("certificate chain was issued by an authority that is not trusted", StringComparison.OrdinalIgnoreCase) >= 0)
+                {
+                    AnsiConsole.MarkupLine("[yellow]Troubleshooting tips:[/]");
+                    AnsiConsole.MarkupLine("- Ensure SQL Server uses a certificate trusted by this machine's Trusted Root store.");
+                    AnsiConsole.MarkupLine("- Connect using a DNS name that matches the certificate's CN/SAN.");
+                    AnsiConsole.MarkupLine("- Or set [teal]--hostname-in-certificate[/] to the certificate subject.");
+                    AnsiConsole.MarkupLine("- For dev only, use [teal]--trust-server-certificate true[/] to bypass validation.");
+                    AnsiConsole.MarkupLine("- If name keeps flipping to an IP, try [teal]--no-tnir[/].");
+                }
             }
 
         }
 
+        private static bool LooksLikeIp(string server)
+        {
+            // Accept formats: "x.x.x.x" or "x.x.x.x,port"
+            var parts = server.Split('\\')[0]; // ignore instance suffix
+            var ipAndPort = parts.Split(',');
+            var ip = ipAndPort[0].Trim();
+            return Regex.IsMatch(ip, @"^\d{1,3}(\.\d{1,3}){3}$");
+        }
+
         public PingCommand(ILogger<PingCommand> logger)
         {
             Logger = logger;
diff --git a/Commands/Settings/ConsoleSettings.cs b/Commands/Settings/ConsoleSettings.cs
@@ -62,6 +62,24 @@ public override ValidationResult Validate()
         [DefaultValue(10)]
         public int Wait { get; set; }
 
+        // TLS-related options
+        [CommandOption("--encrypt <true|false|strict>")]
+        [Description("Encrypt mode: true|false|strict. Default follows Microsoft.Data.SqlClient (true).")]
+        public string? Encrypt { get; set; }
+
+        [CommandOption("--trust-server-certificate|-T <true|false>")]
+        [Description("Set TrustServerCertificate=true (DEV ONLY). Bypasses cert validation but keeps encryption.")]
+        public bool? TrustServerCertificate { get; set; }
+
+        [CommandOption("--hostname-in-certificate|-H <name>")]
+        [Description("Override HostNameInCertificate for TLS validation (use the subject/SAN on the server certificate).")]
+        public string? HostNameInCertificate { get; set; }
+
+        [CommandOption("--no-tnir")]
+        [Description("Disable TransparentNetworkIPResolution to avoid host->IP substitution that breaks TLS name matching.")]
+        [DefaultValue(false)]
+        public bool NoTransparentNetworkIPResolution { get; set; } = false;
+
 
 
         //[CommandOption("--command-option-value <value>")]
diff --git a/README.md b/README.md
@@ -15,14 +15,49 @@ This is a .NET console app and should run fine on [all supported operating syste
     -h, --help                          Prints help information
     -d, --database           master     Database name
     -u, --username                      SQL Username, leave empty for Windows integrated security
-    -p, --password                      Password
+    -p, --password                      Password (NOT RECOMMENDED - see Security Notes below)
     -t, --timeout            3          Connection timeout in seconds
     -c, --command                       SQL Command (default will return database information)
     -f, --failover                      This is a failover cluster / availability group, use MultiSubnetFailover=true
     -a, --failoverpartner               Use a custom failover partner
     -n, --nonstop                       Set this to true to continously ping the server. Default is ping once
     -w, --wait <SECONDS>     10         How long to wait, in seconds, between non-stop pings
 
+## SECURITY NOTES:
+
+### Credential Handling
+
+SQLPing supports multiple ways to provide credentials, listed from most secure to least secure:
+
+**1. Interactive Password Prompt (RECOMMENDED)**
+```bash
+# Provide username, password will be prompted securely
+SQLPing myserver -u myusername
+Password: ********
+```
+
+**2. Environment Variables (RECOMMENDED)**
+```bash
+# Set environment variables (more secure than command line)
+export SQLPING_USERNAME="myusername"
+export SQLPING_PASSWORD="mypassword"
+SQLPing myserver
+```
+
+**3. Command Line Arguments (NOT RECOMMENDED)**
+```bash
+# WARNING: Password visible in process list and shell history
+SQLPing myserver -u myusername -p mypassword
+```
+
+**Important Security Considerations:**
+- Passwords provided via command line (`-p`) are visible in:
+  - Shell history
+  - Process lists (`ps aux`, Task Manager)
+  - Logs and monitoring tools
+- Always prefer environment variables or interactive prompts for production use
+- Connection strings in output have passwords redacted automatically
+
 
 # Todo
 
diff --git a/SQLServerPing.csproj b/SQLServerPing.csproj
