-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Remove approval prompt from dotnet tool exec - running dnx is implicit approval
#52956
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
base: release/10.0.3xx
Are you sure you want to change the base?
Conversation
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
… logic Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
dotnet tool exec
|
@copilot the entire "check for approval" concept is unnecessary. The fix here should be to remove the entire "prompt" check - the usage of |
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Agreed - I've removed the entire approval check. Running |
dotnet tool execdotnet tool exec - running dnx is implicit approval
dsplaisted
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with this, but AFAIK npm does prompt you when in interactive mode. I'm not sure everyone was fully agreed to remove the prompt entirely.
| { | ||
| if (_interactive) | ||
| { | ||
| Reporter.Error.WriteLine(CliCommandStrings.ToolDownloadCanceled.Red().Bold()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Also remove the CliCommandStrings that have been removed from the resources. You'll probably need to do a build to update the xlf files too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the unused resource strings (ToolDownloadConfirmationPrompt, ToolDownloadCanceled, ToolDownloadNeedsConfirmation) and updated all xlf files using /t:UpdateXlf. Changes in commit 4744ace.
Co-authored-by: dsplaisted <145043+dsplaisted@users.noreply.github.com>
You're right that npm does prompt in interactive mode. However, @baronfel's guidance was that for If the team prefers npm's behavior (prompt in interactive, auto-accept in non-interactive), I can restore the prompt logic but fix it to properly detect non-interactive scenarios. Let me know which approach you'd prefer. |
Remove approval prompt from
dotnet tool exec- running dnx is implicit approvaldotnet tool exec(dnx) previously prompted users to confirm tool downloads, blocking LLMs, scripts, and automation. This PR removes the approval prompt entirely - runningdnxis now treated as implicit approval.Changes
Removed approval check
UserAgreedToRunFromSource()method that prompted for confirmationERROR_CANCELLEDconstant and_interactivefieldToolDownloadConfirmationPrompt,ToolDownloadCanceled,ToolDownloadNeedsConfirmationRationale
When a user runs
dotnet tool exec <package>, they are explicitly requesting to execute that package - this is the approval. The previous confirmation prompt:This aligns with the philosophy of tools like
npxwhere running the command implies consent to download and execute the requested package.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.