Warn before the user runs a new commandline elevated#11308
Warn before the user runs a new commandline elevated#11308zadjii-msft wants to merge 124 commits intomainfrom
Conversation
…inal-content-elevation-warning
…ked raymond chen for help.
Sublime can delete the file just fine (and write any old file in it's place) Even we can modify the file when running unelevated, which is **B**ad. So there's gotta be some way to allow _us_ to write the file only when elevated
…'t care about elevation or none of that, so the unelevated terminal could atomically rewrite the elevated file, which isn't what we wanted.
…inal-content-elevation-warning
…on-terminal-content-elevation-warning
(cherry picked from commit b499d44d4baf21c279dbb9f3a766bc9c37528b62)
miniksa
left a comment
There was a problem hiding this comment.
There's still a lot of TODOs here... do you intend to finish those before merge?
src/cascadia/LocalTests_TerminalApp/TerminalApp.LocalTests.vcxproj
Outdated
Show resolved
Hide resolved
## Summary of the Pull Request This creates an `elevated-state.json` that lives in `%LOCALAPPDATA%` next to `state.json`, that's only writable when elevated. It doesn't _use_ this file for anything, it just puts the framework down for use later. It's _just like `ApplicationState`_. We'll use it the same way. It's readable when unelevated, which is nice, but not writable. If you're dumb and try to write to the file when unelevated, it'll just silently do nothing. If we try opening the file and find out the permissions are different, we'll _blow the file away entirely_. This is to prevent someone from renaming the original file (which they can do unelevated), then slapping a new file that's writable by them down in it's place. ## References * We're going to use this in #11096, but these PRs need to be broken up. ## PR Checklist * [x] Closes nothing * [x] I work here * [x] Tests added/passed * [ ] Requires documentation to be updated - maybe? not sure we have docs on `state.json` at all yet ## Validation Steps Performed I've played with this much more in `dev/migrie/f/non-terminal-content-elevation-warning` ###### followed by #11308, #11310
Yea, just wanted to push to get it on the cloud before the weekend. Between the three PRs in this chain they'll all get cleaned up. |
…inal-content-elevation-warning
|
So, in the recent 1.13 selfhost builds of Terminal I've been seeing all sorts of weird sizing issues -- sometimes when I launch unpackaged I will get a normal-sized window with a one-column-wide TermControl: Other times -- you know that inverse block of text when powershell starts up telling you about a new version? That looks like it's been resized down and is no longer square (I'll screenshoot next time it happens.) I wonder if that's related to the changes here that make panes no longer host TermControls? Further: if you right-click a tab that's displaying an elevation warning and choose "split," terminal crashes. |
|
There it is- It's like the opposite of #32! |
Fix it in post? Couldn't get it repro'd this morning 😕 Were you launching unpackaged, elevated, hitting the dialog, then accepting it? Or just elevated (no prompt)? Or unelevated?
That's fixed in 056446c |
This comment has been minimized.
This comment has been minimized.
| } | ||
| // Always look in "%LOCALAPPDATA%\Microsoft\WindowsApps", which is | ||
| // where the pwsh.exe execution alias lives. | ||
| { wil::ExpandEnvironmentStringsW<std::wstring>(L"%LOCALAPPDATA%\\Microsoft\\WindowsApps") }, |
There was a problem hiding this comment.
Sorry to be a stick in the mud, but unfortunately this path is user-controlled.
(dhowett-sl) ~\src\wt-1.13 % echo hi > $Env:LOCALAPPDATA\Microsoft\WindowsApps\FUN.EXE
(dhowett-sl) ~\src\wt-1.13 % FUN
ResourceUnavailable: Program 'FUN.EXE' failed to run: The specified executable is not a valid application for this OS platform.
Unelevated, but weirdly I was hitting an issue where OpenConsole.exe wouldn't run. Try running unpackaged, deleting OpenConsole, and then opening wt unelevated. It was a solid 100% repro for my terminal window being the wrong size. |
## Summary of the Pull Request This is the resurrection of #8514 and #11310. WE determined that we didn't want to do #11308 after all, so this should be profile auto-elevation, without the warning. This PR adds two features: * the `elevate: bool` property to profiles - If the user is running unelevated, **and** `elevate` is set to `true`, then instead of opening a new tab, we'll open an elevated Terminal window with the profile. - Otherwise, we'll just open a new tab in the existing window. This includes cases where the window is elevated, and the profile is set to `elevate:false`. `elevate:false` basically just means "do nothing special with me". * the `elevate: bool?` property to `NewTerminalArgs` (`newTab`, `splitPane`) - This allows a user to create an action that will elevate the profile, even if the profile is not otherwise set to auto-elevate. - `elevate:null` (_the default_) does not change the profile's elevation status. The action will use whatever is set by the profile. - `elevate:true` will attempt to auto-elevate the profile - `elevate:false` will do nothing special. ## References * #5000 for obvious reasons * Spec'd in #8455 ## PR Checklist * [x] Closes #632 * [x] I work here * [x] Tests added/passed * [ ] Requires documentation to be updated - sure does, but that'll come all at the end. ## Detailed Description of the Pull Request / Additional comments After playing with de-elevation a bit, it turns out it behaves weirdly with packaged applications. I can try and ask `explorer.exe` to launch the process on our behalf. However, if the thing we're launching is an execution alias (`wt.exe`), and we're elevated, then the child process will _still launch elevated_. There's also something super BODGEY at work here. `ShellExecute` is the function we use to ask the OS to elevate something for us. But `ShellExecute` needs to be able to send a window message to the process that called it (if the caller was a WINDOWS subsystem application). So if we die immediately after calling `ShellExecute`, then the elevated process never actually spawns - sad. So we're adding a helper process, `elevate-shim.exe`, that lives in our process. That'll be the one that actually calls `ShellExecute`, so that it can live for the duration of the UAC prompt. ## Validation Steps Performed * Ran tests * Opened a bunch of terminal tabs at various different elevation levels * opened new splits too * In the defaults (base layer) as well, for madness Some settings to use for testing <details> ```jsonc "keybindings" : [ ////////// ELEVATION /////////////// { "keys": "f1", "name": "ELEVATED TAB", "icon": "\uEA18", "command": { "action": "newTab", "elevate": true } }, { "keys": "f2", "name": "ELEVATED, Color", "icon": "\uEA18", "command": { "action": "newTab", "elevate": true, "commandline": "PowerShell.exe", "startingDirectory": "C:\\Windows", "tabColor": "#bbaa00" } }, { "keys": "f3", "name": "unelevated ELEVATED", "icon": "🙃", "command": { "action": "newTab", "elevate": false, "profile": "elevated cmd" } }, ////////////////////////////// ], "profiles": { "defaults": { "elevate": true, }, "list": [ { "hidden":false, "name" : "cmd", "commandline" : "cmd.exe", "guid": "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}", "startingDirectory" : "%USERPROFILE%", "opacity" : 20 }, { "name" : "the COOLER cmd", "commandline" : "c:\\windows\\system32\\cmd.exe", "startingDirectory" : "%USERPROFILE%", }, { "name" : "the sneaky cmd", "commandline" : "c:\\windows\\system32\\cmd.exe /k echo sneaky sneaks", "startingDirectory" : "%USERPROFILE%", }, { "name": "elevated cmd", "commandline": "cmd.exe /k echo This profile is always elevated", "startingDirectory" : "well this is garbage", "elevate": true, "background": "#9C1C0C", "tabColor": "#9C1C0C", "colorScheme": "Desert" }, { "name": "unelevated cmd", "commandline": "cmd.exe /k echo This profile is just as elevated as you started with", "elevate": false, "background": "#1C0C9C", "tabColor": "#1C0C9C", "colorScheme": "DotGov", "useAcrylic": true }, ] ``` </details> Also try: * `wtd nt -p "elevated cmd" ; sp -p "elevated cmd"` * `wtd nt -p "elevated cmd" ; nt -p "elevated cmd"` This was merged manually via ``` git diff dev/migrie/f/non-terminal-content-elevation-warning dev/migrie/f/632-on-warning-dialog > ..\632.patch git apply ..\632.patch --ignore-whitespace --reject ```

targets #11222, followed by #11310
Summary of the Pull Request
As a part of #8455, we identified that we should probably warn before running commandlines, to make sure the user knows what they're about to do. This dialog looks like the following:
When the user approves a commandline, we'll remember that commandline, so they won't get prompted every time.
If they reject a commandline, then we're just going to close that pane.
References
PR Checklist
state.json#11096Detailed Description of the Pull Request / Additional comments
This PR changes
Paneto be able to host anyUserControl, not necessarily aTermControl. This of course has bigger repurcussions than just revealed here. It's gonna wildly conflict with some of the other open PRs. I wanted to do this more generically for #997, but alas, nobody got time for that.This adds a
AdminWarningPlaceholderwhich is a type ofUserControlthat just holds anotherUserControl. It looks just like aContentDialog, but will actually resize as the window resizes.We won't prompt for certain commandlines:
C:\windows\system32\cmd.exe.cmd.exewill prompt, because it's unqualified.C:\windows\system32\cmd.exe /k echo sneaky sneakwill also prompt, because it's got other args.%SystemRoot%\System32\cmd.exewon't prompt, because it's smart enoguh to handle env vars%SystemRoot%\System32\wsl.exe -d <distroname>won't prompt, because we trust wsl distros.%SystemRoot%\System32\wsl.exe -d <distroname> bash -c do-malicious-shit.shWILL promptValidation Steps Performed
Opened a bunch of terminals, in random orders, and made sure that the above commandlines would work as expected. My list of running profiles:
Details
{ "commandline": "%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", "guid": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}", "hidden": false, "name": "Windows PowerShell" }, { "commandline": "%SystemRoot%\\System32\\cmd.exe", "guid": "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}", "hidden": false, "name": "Command Prompt" }, { "guid": "{574e775e-4f2a-5b96-ac1e-a2962a402336}", "hidden": false, "name": "PowerShell", "source": "Windows.Terminal.PowershellCore" }, { "guid": "{c6eaf9f4-32a7-5fdc-b5cf-066e8a4b1e40}", "hidden": false, "name": "Ubuntu-18.04", "source": "Windows.Terminal.Wsl" }, { "commandline": "c:\\windows\\system32\\cmd.exe", "guid": "{5aea3919-92fa-5990-bb39-2321f316d9b9}", "name": "the COOLER cmd", "startingDirectory": "%USERPROFILE%" }, { "commandline": "c:\\windows\\system32\\cmd.exe /k echo sneaky sneaks", "guid": "{4fd85e9a-919a-5033-9118-1b7518b676a7}", "name": "the sneaky cmd", "startingDirectory": "%USERPROFILE%" }, { "background": "#9C1C0C", "commandline": "cmd.exe /k echo This profile is always elevated", "guid": "{7a7854d2-65bc-57c2-a2c6-70a32a2f600e}", "name": "elevated cmd", "startingDirectory": "well this is garbage", "tabColor": "#9C1C0C" }, { "background": "#1C0C9C", "commandline": "cmd.exe /k echo This profile is just as elevated as you started with", "guid": "{b5c1cbf5-217f-5e0f-b3ee-3c70150ef037}", "name": "unelevated cmd", "tabColor": "#1C0C9C", "useAcrylic": true },