-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dotnet dev-certs https
no longer creates a folder if one does not exist
#58330
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
1 similar comment
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
dev-certs is a bundled global tool that comes into dotnet/installer. dev-certs comes from the aspnetcore repo so I am not sure the sdk repo is the best place for this issue...moving. |
Looks like #57108 changed this area |
/cc @amcasey |
Just commenting on this, that our builds are now failing with this exact same error on GitHub Actions. |
That was an intentional change, made to improve security. The reasoning was that, if the tool created a directory, it would have to choose file access permissions on the user's behalf and that maximally conservative settings would break some apps. We decided instead to delegate that decision back to the user. I'm sorry that you spent a bunch of time debugging this. My personal recommendation is to always pass |
If this is common enough and we maintain our position on directory creation, we may want to have special output in this case that doesn't require |
@amcasey is there a suggested method/workaround we should use moving forward? is OP's work around the most ideal? |
Yes, the new expected flow is |
@tibble49, @sironjuh, @cgdallas, @Gregory-Ledray Can you help me understand how we can make the error experience more helpful? Putting aside that it's annoying to have the behavior of a tool suddenly change underneath you (sorry), what was it about the experience that made it unclear how to recover (i.e. by adding an explicit mkdir)? Did you see the verbose output and find it unclear or did you not see that output at all and just hit the generic "export failed" message? |
I don't think it was unclear. It looks like it took me between 6 and 22 minutes to fix the problem. The error I opened an Issue because this is a change in behavior I didn't expect, so I assumed it was a bug. |
In our case, we were using an out-of-the-box aspnet SPA template via
So I don't know how the dotnet tooling would have been able to surface something since the Node process is the one that ran into the issue. Had we seen an error similar to what Gregory mentioned above, that would have been way more clear as to what happened though - I'm just not sure its feasible. |
Totally fair point and we appreciate it. 😄 In this case, it happens to have been expected, but keep filing those bugs. |
@cgdallas I didn't know there was a SPA template that depended on exporting the dev cert - I'll give that a shot. The template may need to be updated to do the directory creation. It's also possible we can change the error surfaced from node - I think |
@cgdallas Can I bug you for repro steps? I did the trivial |
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 in dotnet#57108, which included a backport of dotnet#56985, and shipped in 8.0.10. For dotnet#58330
Hey @amcasey Correct that we encountered this issue as part of Docker container build being run by a Github Action. FWIW, I just tried a So I guess I can't say with 100% certainty how it was created since it predates me. I (incorrectly) assumed it was just a And just for other references, this was the Vite issue where the possibility of my error being a dotnet issue came from: |
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 in #57108, which included a backport of #56985, and shipped in 8.0.10. For #58330
@cgdallas Thanks! I think that template had to change pretty regularly to stay up-to-date with react. However, I checked our docs and it looks like it may no longer be supported. Nevertheless, I got the info I needed and I've posted a PR (no promises). |
Glad to hear it - we ended up fixing up our Vite build process to not run if it's running in the CI pipeline, there didn't seem much point in having dev-certs run for those builds anyways. |
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 in #57108, which included a backport of #56985, and shipped in 8.0.10. For #58330
* Improve dev-certs export error message 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 in #57108, which included a backport of #56985, and shipped in 8.0.10. For #58330 * Improve error test
* Improve dev-certs export error message 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 in #57108, which included a backport of #56985, and shipped in 8.0.10. For #58330 * Improve error text --------- Co-authored-by: Andrew Casey <[email protected]>
@amcasey did you also check out this template here? https://learn.microsoft.com/en-us/visualstudio/javascript/tutorial-asp-net-core-with-react?view=vs-2022 This template is the one that splits the project into a Project.Client and Project.Server similar to how @cgdallas described. |
Thanks! That does look more similar to what was described above. You need to enable container support to trigger cert export, which makes sense. I'm not sure yet whether the project is doing that or the VS tooling (I'm guessing the latter). |
I have an app that executes and npm build via This Thankfully a search brought me here and I also disabled the dev cert creation during CI. |
Thanks for reporting, I stumbled on that issue today, and it would have taken me a while to figure out both using |
@seangwright @mderriey Thanks for the reports! The next patch should include a better error message. |
The template has already been updated and should have the appropriate mkdir in the next VS 17 patch. (vite.config.js in the client project does an export.) However, it looks like there's a second export somewhere in the VS tooling - I'm still tracking it down. It may not matter if it runs after the template code that exports to the same folder. |
* Improve dev-certs export error message 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 in #57108, which included a backport of #56985, and shipped in 8.0.10. For #58330 * Improve error text --------- Co-authored-by: Andrew Casey <[email protected]>
I ran into this bug as well. This thread helped me resolve it. Unfortunately, if this is the "new project" experience for someone evaluating .NET, it's not pretty. |
@EdCharbeneau Which project template did you use? The React + ASP.NET Core one should be fixed in the next VS 2022 patch and I couldn't find others that appeared to be affected, but please let me know if I missed one. |
Yes, it was the ASP + React template. |
Well then, I'm sorry for the trouble, but the fix will be available shortly. |
Awesome to see this getting fixed quickly! |
it can be fixed by calling mkdir manually:
|
I am wondering why was this working before but then it stopped? Becuase the above code change fixed the issue. |
is this reduce capability announced anywhere? seems like its breaking our configuration and need to add extra checking |
Describe the Bug
dotnet dev-certs https
no longer creates a folder if one does not existSteps to Reproduce
Previously, running the following code worked:
Now, it produces an error:
Workaround - this works:
Other Information
Deployment at Oct 8, 2024 10:12 AM (UTC-5:00) worked
Deployment at Oct 9, 2024 11:21 AM (UTC-5:00) failed
Working output
Failed output
Output of
docker version
N/A happened on both Windows Docker Desktop & in Ubuntu CI build
Output of
docker info
N/A happened on both Windows Docker Desktop & in Ubuntu CI build
The text was updated successfully, but these errors were encountered: