Handle missing certutil during cert cleanup - #18580
Conversation
Treat NSS cleanup as best effort when certutil is unavailable so certificate cleanup can continue removing the development certificate. Add a Linux doctor warning for missing certutil and tests for both diagnostics and cleanup behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18580Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18580" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR makes Linux certificate cleanup resilient to a missing certutil tool. Previously, when certutil was unavailable, UnixCertificateManager.RemoveCertificateFromTrustedRoots treated each NSS database as an untrust failure and threw an InvalidOperationException, causing aspire certs clean to fail. Since NSS (Firefox/Chromium) cleanup is best-effort, the change now logs the missing tool and skips the NSS loop without flagging a hard failure. It also surfaces a localized warning in aspire doctor on Linux when certutil is missing, and extracts the certutil command name plus a reusable IsCommandAvailable helper into CertificateHelpers.
Changes:
- Skip NSS cleanup (instead of failing) in
RemoveCertificateFromTrustedRootswhencertutilis unavailable. - Add a Linux-only
aspire doctorwarning (dev-certs-certutil) with localized message/details/fix and updated.resx/.Designer.cs/.xlfresources. - Centralize
CertUtilCommandand add a staticIsCommandAvailable(command, environment)helper inCertificateHelpers, plus regression and doctor-check tests.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Certificates/CertificateGeneration/UnixCertificateManager.cs | Core behavior change: skip NSS untrust loop when certutil missing; reference shared CertificateHelpers.CertUtilCommand. |
| src/Aspire.Cli/Certificates/CertificateHelpers.cs | Adds CertUtilCommand const and static IsCommandAvailable PATH-scan helper. |
| src/Aspire.Cli/Utils/EnvironmentChecker/DevCertsCheck.cs | Adds Linux dev-certs-certutil warning result when certutil is absent. |
| src/Aspire.Cli/Resources/DoctorCommandStrings.resx | New message/details/fix strings for the missing-certutil warning. |
| src/Aspire.Cli/Resources/DoctorCommandStrings.Designer.cs | Generated accessors for the new strings. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.*.xlf (13 locales) | XLF entries (state="new") for the new strings. |
| tests/Aspire.Cli.Tests/Certificates/UnixCertificateManagerTests.cs | New regression test verifying removal does not throw when certutil/NSS dbs are present but certutil is missing. |
| tests/Aspire.Cli.Tests/Utils/DevCertsCheckTests.cs | Tests that the doctor warning is emitted only when certutil is missing on Linux. |
Files not reviewed (1)
- src/Aspire.Cli/Resources/DoctorCommandStrings.Designer.cs: Generated file
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Dogfood artifact verificationObjective: Verify the tested CLI came from PR #18580 at the expected head commit. Steps:
Evidence:
Observations:
Scenario 2: Basic CLI/template smokeObjective: Verify the PR CLI and PR package hive can create a fresh Aspire project. Steps:
Evidence:
Observations:
Scenario 3: Missing certutil cleanup regressionObjective: Verify Linux certificate cleanup treats missing certutil as best effort when NSS DB paths exist. Steps:
Evidence:
Observations:
Expected Unhappy-Path Outcome: Cleanup remains successful because NSS cleanup is best effort. Scenario 4: Doctor missing certutil warningObjective: Verify Linux doctor reports a warning when certutil is unavailable. Steps:
Evidence:
Observations:
Expected Unhappy-Path Outcome: Missing certutil is reported as a warning, not as a failed certificate check. Summary
Overall Result✅ PR VERIFIED Recommendations
|
James Newton-King (JamesNK)
left a comment
James Newton-King (JamesNK)
left a comment
There was a problem hiding this comment.
One suggestion to reduce code duplication in the PATH-scanning logic.
James Newton-King (JamesNK)
left a comment
James Newton-King (JamesNK)
left a comment
There was a problem hiding this comment.
One suggestion to reduce code duplication in the PATH-scanning logic.
Refactor cert command availability checks to use the shared CertificateHelpers implementation for both doctor warnings and Unix certificate management. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 98 / 98 test projects · 7 jobs, from 22 changed files. Selected test projects (98 / 98)
Selected jobs (7)
How these were chosen — grouped by what changed
🔧 show 97
📦 affected project 📦 affected project 🧪 🧪 🧪 Job reasons
Selection computed for commit |
|
Documentation was required for this PR (triggered signals: See the workflow run for details: https://github.com/microsoft/aspire/actions/runs/28542343293 |
Description
Linux certificate cleanup should not fail just because
certutilis unavailable. NSS database cleanup is best effort, so this change logs the missing tool and continues with the rest of certificate cleanup instead of treating browser cleanup as a hard failure.This also updates
aspire doctoron Linux to report a warning whencertutilis missing, since Firefox and Chromium browser certificate trust may be incomplete without NSS tools. The warning uses localized CLI resources and the generated XLF files were updated.User-facing usage
When
certutilis missing on Linux,aspire certs cleancan still complete cleanup for the available certificate stores.aspire doctornow reports a warning explaining that browser certificate trust may be incomplete and suggests installing the distribution NSS tools package, for examplelibnss3-tools.Validation:
Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?