Skip to content

Ensure SSL_CERT_DIR messages are always shown and check for existing value#13

Open
tomerqodo wants to merge 6 commits intoaugment_combined_20260121_augment_sentry_coderabbit_1_base_ensure_ssl_cert_dir_messages_are_always_shown_and_check_for_existing_value_pr82from
augment_combined_20260121_augment_sentry_coderabbit_1_head_ensure_ssl_cert_dir_messages_are_always_shown_and_check_for_existing_value_pr82
Open

Ensure SSL_CERT_DIR messages are always shown and check for existing value#13
tomerqodo wants to merge 6 commits intoaugment_combined_20260121_augment_sentry_coderabbit_1_base_ensure_ssl_cert_dir_messages_are_always_shown_and_check_for_existing_value_pr82from
augment_combined_20260121_augment_sentry_coderabbit_1_head_ensure_ssl_cert_dir_messages_are_always_shown_and_check_for_existing_value_pr82

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#82

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Jan 22, 2026

🤖 Augment PR Summary

Summary: This PR improves how dotnet-dev-certs reports OpenSSL trust guidance on Unix.

Changes:

  • Updated OpenSSL/SSL_CERT_DIR guidance messages and added events for “already configured” and “append to variable” cases.
  • Enhanced Unix trust flow to detect whether SSL_CERT_DIR is already set and contains the dev-certs directory.
  • Ensured SSL_CERT_DIR-related messages are emitted without requiring --verbose by always registering a reporter event listener and enabling LogAlways events.
  • Refactored NssDb away from primary-constructor syntax.

Technical Notes: Non-verbose runs now subscribe to EventLevel.LogAlways for CertificateManager.Log so users consistently see environment variable instructions.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

if (!string.IsNullOrEmpty(existingSslCertDir))
{
var existingDirs = existingSslCertDir.Split(Path.PathSeparator);
var certDirFullPath = Path.GetFullPath(prettyCertDir);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

certDirFullPath is derived from prettyCertDir, which can contain the literal $HOME prefix; that makes Path.GetFullPath(prettyCertDir) unlikely to match the real absolute paths typically stored in SSL_CERT_DIR, so isCertDirIncluded may be false even when correctly configured.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

hasValidSslCertDir = false;
}

sawTrustFailure = !hasValidSslCertDir;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment overwrites any earlier sawTrustFailure state (e.g., NSS trust failures above), which can change the final TrustLevel from partial to full (or vice-versa) based solely on SSL_CERT_DIR configuration.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants