- Notifications
You must be signed in to change notification settings - Fork10.5k
[release/8.0] Improve dev-certs export error message#58470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
During a recent security review of the dev-certs tool, we observed that on export it would create a directory that was potentially world-readable (e.g. based on permissions inherited from the parent directory). We decided it would be more appropriate to let users make the decision of who should have access to the directory. Unfortunately, this removal of functionality broke some app authors' workflows. When dev-certs is run directly, the `--verbose` output makes it clear what went wrong and what needs to happen, but the non-verbose output that appears when another tool does the export is less helpful. This change introduces a new top-level error state for an export failure caused by a non-existent target directory to make it clearer how to fix broken workflows.The behavior changed indotnet#57108, which included a backport ofdotnet#56985, and shipped in 8.0.10.Fordotnet#58330
amcasey commentedOct 16, 2024
Per discussion in#58330, the issue was not that the verbose error message - which already mentions the missing directory - was unclear but that upstream tooling might not display the verbose output. I'm a little reluctant to add a new error condition for what is hopefully a point-in-time problem, but the behavior change happened in a patch and I expect this failure mode to be quite common. |
amcasey commentedOct 16, 2024
/backport to release/9.0 |
Started backporting to release/9.0:https://github.com/dotnet/aspnetcore/actions/runs/11372697592 |
amcasey commentedOct 16, 2024
Personally, I think the 8.0 version of this change is the most important, since the behavior changed patch-to-patch. A change in a major version will be less surprising (even though we don't use semver). |
Uh oh!
There was an error while loading.Please reload this page.
amcasey commentedOct 16, 2024
Approved over email |
b74d4a6 intodotnet:release/8.0Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Improve dev-certs export error message
Mention non-existent target directory
Description
During a recent security review of the dev-certs tool, we observed that on export it would create a directory that was potentially world-readable (e.g. based on permissions inherited from the parent directory). We decided it would be more appropriate to let users make the decision of who should have access to the directory. Unfortunately, this removal of functionality broke some app authors' workflows. When dev-certs is run directly, the
--verboseoutput makes it clear what went wrong and what needs to happen, but the non-verbose output that appears when another tool does the export is less helpful. This change introduces a new top-level error state for an export failure caused by a non-existent target directory to make it clearer how to fix broken workflows.The behavior changed in#57108, which included a backport of#56985, and shipped in 8.0.10.
For#58330
Customer Impact
Development certificate export fails. This is most likely to break scenarios where the app is being developed (or validated in CI) in a container.
Regression?
8.0.8
Risk
A slightly different string is printed - everything else stays the same.
Verification
Packaging changes reviewed?