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

Targeting current instance with "-w 0" command line doesn't work if running as administrator #9628

Open
DJackman123 opened this issue Mar 26, 2021 · 21 comments
Labels
Area-Remoting Communication layer between windows. Often for windowing behavior, quake mode, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Tracking-External This bug isn't resolved, but it's following an external workitem.
Milestone

Comments

@DJackman123
Copy link
Contributor

Windows Terminal version (or Windows build number)

1.7.572.0

Other Software

No response

Steps to reproduce

Launch Windows Terminal as administrator
Run the command line: wt -w 0 nt

Expected Behavior

This should open a new tab in the current window

Actual Behavior

Launches a new instance of Windows Terminal

It would make sense that I wouldn't be able to send commands to other WT windows that were running as admin, but running a command specifically targeting the current window should still work (as a special case if necessary).

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 26, 2021
@zadjii-msft
Copy link
Member

Weird, because I definitely can't repro this locally. @miniksa Do you know if there's a way I could have @DJackman123 here collect the events emitted to a particular tracelogging GUID and send them to me? That is what all that logging is for after all

@zadjii-msft zadjii-msft added Area-Remoting Communication layer between windows. Often for windowing behavior, quake mode, etc. Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. Product-Terminal The new Windows Terminal. labels Mar 26, 2021
@DHowett
Copy link
Member

DHowett commented Mar 26, 2021

Right, this is when you run wt -w 0 from an application hosted in the admin wt?

@miniksa
Copy link
Member

miniksa commented Mar 26, 2021

Weird, because I definitely can't repro this locally. @miniksa Do you know if there's a way I could have @DJackman123 here collect the events emitted to a particular tracelogging GUID and send them to me? That is what all that logging is for after all

@zadjii-msft, you could ensure that the GUID you're thinking about is listed in src/ConsolePerf.wprp and then have @DJackman123 load that into Windows Performance Recorder with the Add Profiles... button in the bottom left, start the trace, repro the thing, then send you the recorded ETL file.

@DHowett
Copy link
Member

DHowett commented Mar 26, 2021

I've got a version of that file laying around that has Terminal GUIDs in it... Just a sec 😄

@DHowett
Copy link
Member

DHowett commented Mar 26, 2021

@miniksa
Copy link
Member

miniksa commented Mar 26, 2021

Here is a version of the WPR profile that has all the Terminal GUIDs.

Dude. Put that in a PR and check that in.

@DJackman123
Copy link
Contributor Author

DJackman123 commented Mar 26, 2021 via email

@DHowett
Copy link
Member

DHowett commented Mar 26, 2021

huh. I don't love the implications that has on our activation stack.

Hey, are you getting a different config file when you run from the Run dialog?

@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Mar 26, 2021
@DJackman123
Copy link
Contributor Author

What specifically do you mean by "different config file"?

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 26, 2021
@zadjii-msft
Copy link
Member

If you open the Settings file in both the first window that opens, and the second window (the one the subsequent wt -w 0 glommed to), are they the same file at the same path?

@zadjii-msft zadjii-msft added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Mar 26, 2021
@DJackman123
Copy link
Contributor Author

DJackman123 commented Mar 26, 2021 via email

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 26, 2021
@jdhitsolutions
Copy link

I'm glad I finally found this issue. I see the same behavior on 1.8.1521.0 in Windows 10 21H1. Running wt -w 0 nt in a non-elevated Windows Terminal instance opens the new tab in the same instance. Doing the same thing in an elevated instance launches a new Windows Terminal instance, which is not what I want. The new instance is also elevated which is what I would expect.

@DJackman123
Copy link
Contributor Author

So it's not just me? I'm not going crazy??
If you run the wt -w 0 nt command in the new Windows Terminal instance that was started when you ran the command from the first elevated instance, what happens? For me I get a new tab in the second instance. So it works, but just not in the initial elevated instance.

@jdhitsolutions
Copy link

I concur. In my existing, elevated, Windows Terminal instance, when I run wt -w 0 I get a new instance of Windows Terminal, launching my default profile. IN THAT instance, every time I run wt -w 0 nt, it opens a new tab IN THE SAME instance. AND, the new tabs in the second instance are all running elevated. Which I would expect. But otherwise, this behavior makes no sense to me.

@DJackman123
Copy link
Contributor Author

DJackman123 commented Aug 27, 2021 via email

@hardeepparmar
Copy link

hardeepparmar commented Sep 1, 2021

@DJackman123 comment on other thread brought me here.. Yes i can confirm that if the first time terminal is launched with elevated privilege(i.,e Run As administrator) and you issue wt -w 0 nt it opens a new terminal window instance.. However if you again do the same from this new window..it opens(as expected) a tabbed window. However this works fine if you are either using command prompt window for issuing wt -w 0 nt command or if you are not running as administrator. I tried cmd /c wt -w 0 nt from with in PowerShell and even that does not work. I have tried with PW SHELL version 5.x and also 7.2.0-preview 9(latest as of date) and behavior is same. Also i opened PowerShell from with in command prompt tabbed window by typing pwsh OR vice versa (i.e by getting to cmd window from within powershell) and tried the same from there and even that does not work.
See below the first command opened a tabbed PowerShell window while the second one opened it in new instance. Should this be reported to PowerShell team?
windows_terminal_run_as_admin_powershell_tabbed_window

@zadjii-msft zadjii-msft added this to the Terminal v1.14 milestone Feb 14, 2022
@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Attention The core contributors need to come back around and look at this ASAP. labels Feb 14, 2022
@zadjii-msft zadjii-msft added the zInbox-Bug Ignore me! label Mar 10, 2022
@zadjii-msft
Copy link
Member

Huh. Nothing is throwing any errors in this trace. Nothing is obviously wrong (other than the second window being established as the monarch).

  • PID 56056 starts
    • Registers as the COM server for the Monarch class
    • Creates a monarch (its own process)
    • Starts up totally normally.
  • PID 121072 starts
    • Registers as the COM server for the Monarch class
    • Creates a monarch IN ITS OWN PROCESS (here, winrt::create_instance should have used the one that PID 56056 had already registered
    • Starts up normally as if it was the monarch
  • PID 120788 starts
    • Registers as the COM server for the Monarch class
    • Creates a monarch in PID 121072
    • Everything proceeds normally from here on out

Because that second process effectively sniped the Monarch registration, it's performed a coup. Now the first window thinks it's the king but every other window doesn't, so the first window is totally isolated. I'm gonna need to reach out to the COM folks on this one. I did not think that it was possible for one process to snipe the CoRegisterClassObject of another process.

@zadjii-msft zadjii-msft self-assigned this Mar 16, 2022
@zadjii-msft
Copy link
Member

idly: maybe the alias is pointing at a different version of the Terminal? Or like, a different exe? So the first one opens the ....

Nah that doesn't make sense. The Monarch is separated by "SKU", so Dev always talks to the same GUID, Preview always talks on a second GUID, and Release always on a third GUID, and those haven't changed. So if you did run Stable from the Start Menu, and wt.exe was pointing at Preview, then those windows would have wholly different settings. So that's not it.

Presumably, when you're in this state, running the "Identify Window" command gives "Window: 1" for both the windows.
image
image

@zadjii-msft zadjii-msft removed the zInbox-Bug Ignore me! label Mar 24, 2022
@zadjii-msft
Copy link
Member

Filed MSFT:39320463 to track. From the mail thread, this doesn't seem like our fault, but we should still figure it out.

@zadjii-msft zadjii-msft added Tracking-External This bug isn't resolved, but it's following an external workitem. and removed Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. labels Apr 28, 2022
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.14, Backlog Apr 28, 2022
@zadjii-msft zadjii-msft added Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. and removed Needs-Tag-Fix Doesn't match tag requirements labels Apr 28, 2022
@AltitudeApps
Copy link

AltitudeApps commented May 29, 2022

Presumably, when you're in this state, running the "Identify Window" command gives "Window: 1" for both the windows.

Confirmed. Actually, I have 3 windows open. Two are elevated, and one is not. ALL THREE indicate "Window: 1" in response to the Identify Window command from the command palette.

I can confirm that the trick of running wt -w 0 nt again in the window that was launched (but shouldn't have launched) by a wt -w 0 nt command indeed does work, and it launches a new tab in this new window.

So, I guess the workaround will be to immediately issue a wt -w 0 nt command after launching an elevated wt.exe instance, and then close the first window. Things seem to work as expected after that.

Interestingly, I see that doing a wt -w 0 nt in EITHER of the elevated windows will result in a new tab appearing in the second window.

edit: I just reproduced this on a second machine.

@zadjii-msft
Copy link
Member

Okay we got some heavy investigative help on this on. From the internal bug notes:

Flow is,

  1. User launches WindowsTerminal.exe as admin. Has Token A.
  2. WindowsTerminal.exe also launches an OOP COM server with Token A.
  3. Inside, OpenConsole.exe launches
  4. Inside, Powershell.exe launches, but .NET changes the token to add SeDebugPrivilege (for reasons). Token B!

So now user has a normal looking terminal, but the tokens are already in a weird state between window process and shell process.

User tries to open new tab with "wt.exe -w 0 nt" in terminal.

  1. wt.exe launches. With Token B (SeDebugPrivilege)!
  2. It then launches (the same) OOP COM. This should just fetch the existing instance from earlier, but,
  3. COM tries to find existing server
  4. Finds earlier instance
  5. Matches tokens Token A (first WindowsTerminal.exe instance) and Token B (new wt.exe instance).
  6. Tokens are different, so it makes a new OOP server. i.e. new window.

now, if you change your default shell from powershell.exe -> cmd.exe, then the bug goes away!

First. This doesn't repro on Desktop, but does on Centennial because there is an appmodel policy that changes COM's token matching function to be stricter for Centennial than Desktop, AppModelPolicy_ComTokenMatchingForAAAServers_UseNtCompareTokens.

Second. Supposedly the stricter token matching policy is only used because there is no classInfo (IComClassInfo) retrieved for your COM class.

    wil::com_ptr_nothrow<IComClassInfo> classInfo;
    RETURN_IF_FAILED(TryGetClassInfoForActivation(*pActParams, &classInfo));

because there's a bug in COM where they calculate the right thing for packages, but then clobber it by immediately recalculating that thing for Desktop. (I'm fuzzy on this part.)

There's an upstream bug, MSFT:39730477 that this was now duped to.

Major thanks to @jsidewhite and @brialmsft for digging in on the COM side of things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Remoting Communication layer between windows. Often for windowing behavior, quake mode, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Tracking-External This bug isn't resolved, but it's following an external workitem.
Projects
None yet
Development

No branches or pull requests

7 participants