Skip to content

Prompt before launching commandlines when elevated ; cache answers in elevated state.json #11096

@zadjii-msft

Description

@zadjii-msft

This is part of https://github.com/microsoft/terminal/projects/5, and a pre-req for #632. I'm promoting this to a full issue since I've got a lot of edge-case-y work that needs doing and tracking somewhere.

  • Hot reload elevated-state.json
  • Find the right place to store the file
  • Figure out the right way to do the permissions on this file.
    • SYSTEM: GENERIC_ALL, Everyone: GENERIC_READ - Didn't work. Elevated Terminal couldn't write the file.
    • Admins: GENERIC_ALL, Everyone: GENERIC_READ - Didn't work. Sublime could just delete the file and write something else in it's place. Un-elevated Terminal could even write the file!
    • DOMAIN_USER_RID_ADMIN "Admin", not "Admins"?
    • Manual experimentation says "MIGRIE-HOME\Administrators": Full Control + "Everyone": Read seems to work as I want. Now how do I get that?
    • Something else?
    • IT WAS SOMETHING ELSE. WriteFileAtomic was the problem - the rename would discard the elevated only version of the file, then write a new one in it's place. Presumably, this doesn't really matter. We should only be calling into that function from an elevated window, but it's unfortunate that it doesn't prevent it from working when unelevated.
  • Maybe decide to still do WriteUTF8FileAtomic. We don't need the link handling, so I'm not worried about that...
  • Move the isElevated() logic to a common place to make sure we have one implementation
  • Make a call to the common isElevated() in WriteUTF8File*(), so that we can bail early if we're unelevated.
  • Check that the file is only writable by admins when opening. If it isn't, blow it away and recreate it.
  • Use unique_sid and in general, make sure to clean up the things allocated for us.
  • Only pop the dialog when running elevated
  • Don't pop the dialog if we're elevated (but UAC is disabled) (this is probably just Unable to drag/drop a file to terminal with UAC turned off #7754)
  • Don't pop the dialog for commandlines that are literally just a path to a file in C:\windows\system32.
    • The file must be a full path
    • The file must be in C:\windows\system32
  • UHG also resolve environment variables, so %SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe will work.
  • UHG WSL distros too
  • Pop the dialog for panes too.
  • Actually format the dialog message with the commandline
  • Resolve what happens when multiple controls should be added at once (See Ideas A and B below)
    • What happens when you reject the creation of a pane? Probably should just close
    • What happens during startup for this dialog? Do we just immediately go to the Complete state, then process the actions one at a time?
    • What happens when running a wt action in an elevated window?
    • Rejecting the creation of the first&only (or all) tabs should just close the window.
    • What happens if you wt nt ; sp, then reject the first tab? There are no tabs now to split!
    • Approving a commandline in one pane should automatically update the other ones.
  • Don't add the same commandline multiple times
  • There's some weird focus issues - Replacing the content of the pane doesn't update the tab title (and presumably other properties as well)
  • What happens when the user isn't an admin, but runs the terminal as another user that's an admin (that also has the Terminal installed)?
  • Make sure to add a "why am I seeing this" link
  • Add the TermControl's background to the admin warning placeholder's background.
  • After 64d02f2, fix the hot-reloading for elevated windows

Engineering deets

what happens when multiple controls should be added at once

Idea A: for scenarios where we're going to perform multiple actions, check all of them for commandlines. If we find one action that has an unapproved commandline, then ask the user if they want to run all of the actions. They either approve them all, or none of them.

Idea B: Use a fake content dialog as a placeholder control, until the connection is approved.

  • B.1: We use non-terminal content in the Pane. We use a fake ContentDialog as a placeholder for the TermControl, then swap it out with the TermControl if the control is allowed.
    • This is in the branch from c66a566 onwards
    • How do we handle closing? I think in my initial implementation, there was a IPaneContent interface that exposed all the things a TermControl might do.
    • Like, what happens when the dialog is rejected. It raises a Closed()? How does the pane listen for that...
    • I suppose the AdminWarningPlaceholder will be holding the Pane, the
    • NO it won't hold the pane. That's A: bad circular refs; and B: the Panes will toss the controls around to other panes. So that's bad.
  • B.2: We tell the TermControl to display the dialog itself. It doesn't Start() the connection immediately, instead it starts with it's fake contentdialog.
    • How does this work with oop controls? It wouldn't. The ContentProcess needs to be started before the TermControl. So even if we told the TermControl "you need to ask permission", we'd have already started the content process. It would be harder for the Control to then tell the app "yes I have permission for the thing you asked", then pass it the ContentProcess.

Metadata

Metadata

Assignees

Labels

Area-UserInterfaceIssues pertaining to the user interface of the Console or TerminalIssue-TaskIt's a feature request, but it doesn't really need a major design.Product-TerminalThe new Windows Terminal.Resolution-Won't-FixWe're just really obstinate about this. There's probably a good reason.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions