Add connection validation results and surface in PowerShell

[PrzemyslawKlys]

Summary

• add a connection validation factory that emits structured error codes and friendly messages
• surface validation feedback in SQL Server and MySQL PowerShell cmdlets before execution
• add unit tests covering malformed, missing, and unsupported connection string scenarios

Testing

• dotnet test DbaClientX.sln -c Release --framework net8.0

---

Codex Task

[claude]

Claude finished @PrzemyslawKlys's task —— View job

---

Pull Request Review

Reviewing the connection validation enhancements for code quality, bugs, performance, security, and test coverage.

Progress

• Read and analyze all changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Analyze test coverage
• Provide comprehensive feedback

Code Quality & Best Practices ✅

Strengths:

• Well-structured, maintainable design with clear separation of concerns
• Excellent use of modern C# features (records, pattern matching, null-coalescing)
• Comprehensive XML documentation throughout
• Consistent naming conventions and code style
• Good use of factory pattern for provider validation profiles

Areas for improvement:

• The SanitizeExceptionMessage method at DbaConnectionFactory.cs:264 only returns exception type name, which may not provide enough context for debugging
• Consider extracting magic numbers (1024, 65535) to named constants for better maintainability

Potential Bugs & Issues 🟡

Minor issues identified:

1. Thread safety concern: The BuilderCache ThreadLocal is disposed in static constructors, but there's no null check when accessing BuilderCache.Value! at DbaConnectionFactory.cs:119. If disposal occurs, this could throw.

2. Incomplete validation logic: In ValidatePortRange at DbaConnectionFactory.cs:203-222, the method checks multiple port parameter names but doesn't validate if multiple conflicting ports are specified.

3. Exception handling: The catch blocks at DbaConnectionFactory.cs:125-136 might be too broad. Consider more specific exception handling for different malformed connection string scenarios.

Performance Considerations ✅

Good performance characteristics:

• ThreadLocal caching of DbConnectionStringBuilder prevents repeated allocations
• Early validation returns prevent unnecessary processing
• Efficient dictionary lookups for provider aliases and disallowed options
• String comparison using StringComparer.OrdinalIgnoreCase for optimal performance

Potential optimizations:

• Consider caching validation results for identical connection strings if this becomes a hot path
• The RequiredServerAndDatabase collection is recreated for each unknown provider - consider using a cached instance

Security Concerns ✅

Strong security implementation:

• Excellent handling of disallowed MySQL options (AllowLoadLocalInfile, etc.)
• SQLite path traversal protection using IndexOf("..") check
• Port validation prevents binding to privileged/reserved ports (< 1024)
• SSL enforcement for MySQL connections (rejects SslMode=None)
• Exception sanitization prevents information leakage (though it might be too aggressive)

Recommendations:

• Consider adding validation for common SQL injection patterns in connection strings
• The SQLite path validation could be enhanced to use Path.GetFullPath() for more robust directory traversal detection

Test Coverage Assessment 🟡

Good coverage areas:

• Core validation scenarios well tested (missing provider, malformed strings, etc.)
• Error handling paths covered
• PowerShell helper integration tested

Coverage gaps (explaining the 64.56% Codecov score):

• Missing tests for MySQL SSL validation (ValidateMySqlOptions)
• SQLite path traversal validation not covered
• Provider alias normalization edge cases
• Additional validation functions for PostgreSQL and Oracle
• PowerShell cmdlet integration paths (lines 118-121 in both cmdlets)

Recommendations

1. Enhance exception sanitization: Consider returning more meaningful error details while still protecting sensitive information
2. Add comprehensive tests: Cover the missing validation scenarios mentioned above
3. Consider caching: For high-throughput scenarios, consider memoizing validation results
4. Documentation: Add examples to the validation methods showing expected vs. invalid connection strings
5. Security: Consider additional validations for connection timeout values and connection limits

Overall Assessment

This is a well-designed, security-conscious implementation that significantly improves the robustness of connection handling. The code quality is high, security considerations are well-addressed, and the design is extensible. The main area for improvement is test coverage for the remaining validation scenarios.

Recommendation: Approve with minor suggestions for enhanced test coverage and exception handling.

---

[codecov]

Codecov Report

❌ Patch coverage is 71.11111% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.07%. Comparing base (2caf66a) to head (27dc870).

Files with missing lines	Patch %	Lines
DbaClientX.Core/Invoker/DbaConnectionFactory.cs	74.78%	23 Missing and 7 partials ⚠️
DbaClientX.PowerShell/CmdletIInvokeDbaXQuery.cs	0.00%	3 Missing ⚠️
DbaClientX.PowerShell/CmdletInvokeDbaXMySql.cs	0.00%	3 Missing ⚠️
...entX.PowerShell/Communication/PowerShellHelpers.cs	70.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
+ Coverage   37.00%   38.07%   +1.07%     
==========================================
  Files          72       73       +1     
  Lines        4094     4228     +134     
  Branches      952      984      +32     
==========================================
+ Hits         1515     1610      +95     
- Misses       2420     2450      +30     
- Partials      159      168       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.