Ensure SSL_CERT_DIR messages are always shown and check for existing value#3
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| if (!string.IsNullOrEmpty(existingSslCertDir)) | ||
| { | ||
| var existingDirs = existingSslCertDir.Split(Path.PathSeparator); | ||
| var certDirFullPath = Path.GetFullPath(prettyCertDir); |
There was a problem hiding this comment.
Path comparison uses display string with unexpanded $HOME
Medium Severity
The code calls Path.GetFullPath(prettyCertDir) where prettyCertDir may contain the literal string $HOME (a shell variable notation used for display). Since Path.GetFullPath() doesn't expand shell variables, it treats $HOME as a literal directory name, producing an incorrect path like /cwd/$HOME/.aspnet/dev-certs/trust. This causes the comparison against actual paths in SSL_CERT_DIR to always fail when the certificate directory is under the home directory. Users will incorrectly be told to configure SSL_CERT_DIR even when it's already set correctly, and trust level will be reported as Partial instead of Full. The code should use certDir (the actual filesystem path) instead of prettyCertDir.
| hasValidSslCertDir = false; | ||
| } | ||
|
|
||
| sawTrustFailure = !hasValidSslCertDir; |
There was a problem hiding this comment.
Assignment overwrites previous trust failure status
Medium Severity
The line sawTrustFailure = !hasValidSslCertDir; uses assignment instead of a logical OR, which overwrites any previous trust failures (from dotnet trust, OpenSSL trust, or NSS DB trust). If hasValidSslCertDir is true, this resets sawTrustFailure to false, causing the function to return TrustLevel.Full even when earlier trust operations failed. Before this change, the code only logged messages and did not modify sawTrustFailure. The fix is to use sawTrustFailure = sawTrustFailure || !hasValidSslCertDir; to preserve prior failure state.
Benchmark PR from agentic-review-benchmarks#3
Note
UnixCertificateManagernow checksSSL_CERT_DIRfor inclusion of the dev cert directory. Logs when already configured, suggests appending if set but missing, or setting the variable (with example) if unset. Trust result becomes partial when the env var is misconfigured.UnixOpenSslCertificateDirectoryAlreadyConfigured (113)andUnixSuggestAppendingToEnvironmentVariable (114); updates example text for setting the variable.dotnet-dev-certsalways attaches an event listener and enablesLogAlwayslevel when not using--verbose, ensuring guidance messages are shown by default.Written by Cursor Bugbot for commit 07d6e09. Configure here.