Merged
Conversation
Owner
Sire
commented
Oct 22, 2025
Sire
commented
- Add TLS options (encrypt, trust-server-certificate, hostname-in-certificate, disable TNIR)
- Upgrade to .NET 8.0 LTS
- Update to Microsoft.Data.SqlClient
- Update NuGet packages
- More async
- Redact passwords from console output
- Username and password optionally in env variables
- Bug fixes and security improvements
- Added TODO.md to .gitignore to keep private development tasks local - Allows developer to maintain private todo lists without exposing them in the public repository 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Security Fix:
- Replaced string interpolation with parameterized query for database name
- Changed default query to use @DatabaseName parameter placeholder
- Added SqlParameter to safely pass database name value
- Prevents SQL injection attacks via --database argument
Before: WHERE name = '{settings.Database}'
After: WHERE name = @DatabaseName (with SqlParameter)
This fix only affects the default query. Custom queries provided via
--command flag are executed as-is (user responsibility).
Example of previously exploitable input:
SQLPing localhost -d "'; DROP DATABASE master; --"
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…ient Package Update: - Replaced System.Data.SqlClient 4.9.0 (deprecated) with Microsoft.Data.SqlClient 5.2.2 - Updated using statement in PingCommand.cs Benefits: - Microsoft.Data.SqlClient is the actively maintained SQL Server client library - Receives security updates and bug fixes - Supports latest SQL Server features - Better performance and compatibility - System.Data.SqlClient is deprecated and no longer receives updates API compatibility: Microsoft.Data.SqlClient maintains API compatibility with System.Data.SqlClient, so no code changes required beyond the namespace. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Security Enhancement: - Added RedactConnectionString() method to sanitize connection strings - Replaces password values with "***REDACTED***" before display - Updated console output to show redacted connection string Security Issue Fixed: Previously, the full connection string including plaintext passwords was displayed in the console output (line 32). This exposed credentials in several ways: - Terminal scrollback - Screen recordings/screenshots - Logs and monitoring tools - Over-the-shoulder viewing Implementation: - Uses SqlConnectionStringBuilder to safely parse and rebuild connection strings - Only redacts the Password property, leaves other info visible for debugging - Includes try-catch for graceful handling of malformed connection strings Example Output: Before: Password=MySecretP@ssw0rd; After: Password=***REDACTED***; 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Framework Update: - Updated TargetFramework from net6.0 to net8.0 - Updated GitHub Actions workflow to use .NET 8.0.x Benefits: - .NET 8.0 is LTS (Long Term Support) until November 2026 - .NET 6.0 reaches end of support in November 2024 - Performance improvements and new language features - Latest security patches and runtime optimizations - Better cross-platform compatibility Breaking Changes: None expected - .NET 8.0 maintains compatibility with .NET 6.0 applications for most scenarios. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Package Updates: - Microsoft.Extensions.Logging: 6.0.1 → 8.0.0 - Microsoft.Extensions.Logging.Console: 6.0.1 → 8.0.0 - System.Text.Json: 6.0.11 → 8.0.5 - Microsoft.SourceLink.GitHub: 1.1.1 → 8.0.0 Packages kept at current version (already latest stable): - Spectre.Console: 0.49.1 (latest) - Spectre.Console.Cli: 0.49.1 (latest) - Spectre.Console.Cli.Extensions.DependencyInjection: 0.2.0 (latest) - Microsoft.Data.SqlClient: 5.2.2 (latest stable) Benefits: - Compatible with .NET 8.0 runtime - Latest security patches - Performance improvements - Bug fixes and stability enhancements 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Async Operations Added: - Converted CallDatabase() to CallDatabaseAsync() - Changed connection.Open() to await connection.OpenAsync() - Changed ExecuteReader() to await ExecuteReaderAsync() - Changed reader.Read() to await reader.ReadAsync() - Replaced Thread.Sleep() with await Task.Delay() - Updated Spectre.Console Status().Start() to StartAsync() Benefits: - Non-blocking I/O operations - threads not blocked during database calls - Better scalability and resource utilization - Improved responsiveness, especially in nonstop ping mode - Modern async/await pattern throughout the application - Proper async propagation from ExecuteAsync down to database calls Performance Impact: - Single ping: Minimal impact (same total time) - Nonstop mode: Better thread pool utilization - Under load: Significantly better scalability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…hods Security Enhancements: - Added support for environment variables (SQLPING_USERNAME, SQLPING_PASSWORD) - Implemented interactive password prompt when username provided without password - Added security warning when password is provided via command line - Updated README with comprehensive security documentation Implementation Details: 1. Environment Variable Support: - Checks SQLPING_USERNAME and SQLPING_PASSWORD before using CLI args - Provides visual feedback when environment variables are used - More secure than command line arguments 2. Interactive Password Prompt: - Automatically prompts for password if username provided without password - Uses Spectre.Console's Secret() input for hidden password entry - Prevents password from appearing in shell history 3. Security Warnings: - Warns users when password provided via -p flag - Educates about risks (shell history, process lists, logs) - Recommends safer alternatives 4. Credential Priority (lowest to highest): - Environment variables (SQLPING_USERNAME, SQLPING_PASSWORD) - Command line arguments (-u, -p) - Interactive prompt (if username without password) Benefits: - Reduces accidental password exposure - Provides secure alternatives to command line passwords - Maintains backward compatibility with existing usage - Educates users about security best practices Updated README includes: - Three credential methods with security ratings - Code examples for each method - Clear warnings about command line password risks - Best practices for production use 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ficate, disable TNIR); ensure target DB is used; handle missing VIEW SERVER STATE when showing connection info
Sire
deleted the
claude/review-project-practices-011CULPt6CJZr5NwAcwscQoM
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.