Skip to content
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

Detect sandboxed assembly load in NuGet, warn about required action #4461

Merged
merged 4 commits into from
Feb 25, 2022

Conversation

erdembayar
Copy link
Contributor

@erdembayar erdembayar commented Feb 15, 2022

Bug

Fixes: NuGet/Home#9954

Regression? Last working version:

Description

Net framework sandboxing causing hard to reason error for nuget.exe downloaded from internet on Windows OS. Now error is very explicit and clear, added exception handler and new NU5133 error to inform customer about need to unblock nuget.exe.

Adding unit test little bit complicated so I didn't do it, but manually tested it.
Test nuget.exe can be found here, please note it's not MS authenticode signed so ask additional permission when download.

Before:
image

After:
image

Before:
image

After:
image

Error message can be improved, please give your feedback.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception - It's bit complicated, so I manually tested. If it's really necessary, please let me know so I can add it.
    • OR
    • N/A
  • Documentation

@erdembayar erdembayar force-pushed the dev-eryondon-9954-nuget-metadata-unblock branch from 5a96d6b to 5702e8f Compare February 15, 2022 22:34
@@ -5433,4 +5433,7 @@ Please consider migrating '{0}' to `PackageReference` and using the pack targets
You can set the '{1}' environment variable to 'true' to temporarily reenable this functionality.</value>
<comment>Please do not localize `project.json` pack and `PackageReference`. 0 - path, 1 - env var name</comment>
</data>
<data name="Error_NuGetExeNeedsToBeUnblockedAfterDownloading" xml:space="preserve">
<value>NuGet.exe file on path {0} needs to be unblocked after downloading : {1}</value>
Copy link
Contributor Author

@erdembayar erdembayar Feb 15, 2022

Choose a reason for hiding this comment

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

@JonDouglas @NuGet/nuget-client
Currently path to nuget.exe included 2 times, most likely we just need one, but couldn't decide which one to keep. Please advise.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

The path should be the executing .exe path. I can't honestly tell from this screenshot. They look the same just that the C:\ path is the more empowering one for the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra info is:
image
Not sure if users care about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed extra part now it looks like below:
image

@erdembayar erdembayar marked this pull request as ready for review February 15, 2022 23:00
@erdembayar erdembayar requested a review from a team as a code owner February 15, 2022 23:00
@erdembayar erdembayar requested a review from nkolev92 February 16, 2022 00:09
@erdembayar erdembayar changed the title Handle sandboxed assembly load in NuGet Hand sandboxed assembly load in NuGet, warn about required action Feb 16, 2022
@erdembayar erdembayar changed the title Hand sandboxed assembly load in NuGet, warn about required action Detect sandboxed assembly load in NuGet, warn about required action Feb 16, 2022
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

Sorry, I meant to leave a comment yesterday, but appareently it stayed in pending.

@erdembayar erdembayar force-pushed the dev-eryondon-9954-nuget-metadata-unblock branch 2 times, most recently from f5702fb to a39b418 Compare February 17, 2022 17:06
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

LGTM. 👏

You probably need to rebase.

@erdembayar erdembayar force-pushed the dev-eryondon-9954-nuget-metadata-unblock branch from 3301e43 to b4281bd Compare February 18, 2022 02:16
@erdembayar
Copy link
Contributor Author

@NuGet/nuget-client
Please review.

@erdembayar erdembayar requested a review from jeffkl February 18, 2022 19:28
@erdembayar erdembayar merged commit 74c6a23 into dev Feb 25, 2022
@erdembayar erdembayar deleted the dev-eryondon-9954-nuget-metadata-unblock branch February 25, 2022 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants