WT crashes on saving settings when HKCU\Console is write-protected #19995

@zadjii-msft commented on GitHub (May 30, 2023):

but running as a different user does

well THAT is interesting, for sure. Are we talking something like "over the shoulder" running as another user (i.e. you're logged in as "Desktop\Foo" but then run the Terminal as "Desktop\Bar", or as "Desktop\MyAdminAccount")? Or just, you have two users, this repros for one of them, but not the other?

A /Feedback Hub dump might be helpful - if anything, it might link us to an existing bug.

There's... basically none of our code in that stack. That makes me think this might be a platform bug, but which?

A possibly dumb question - did you tap save, like, with a touch screen? Or use a mouse?

misc notes:

MSFT:44808462 - open, the same stack (more or less), but definitely not us

@zadjii-msft commented on GitHub (May 30, 2023): > but running as a different user does well THAT is interesting, for sure. Are we talking something like "over the shoulder" running as another user (i.e. you're logged in as "Desktop\Foo" but then run the Terminal as "Desktop\Bar", or as "Desktop\MyAdminAccount")? Or just, you have two users, this repros for one of them, but not the other? A /Feedback Hub dump _might_ be helpful - if anything, it might link us to an existing bug. There's... basically none of our code in that stack. That makes me think this might be a platform bug, but which? A possibly dumb question - did you _tap_ save, like, with a touch screen? Or use a mouse? --- misc notes: * MSFT:44808462 - open, the same stack (more or less), but definitely not us

claunia commented

@alabuzhev commented on GitHub (May 31, 2023):

you're logged in as "Desktop\Foo" but then run the Terminal as "Desktop\Bar"

Yes.

A possibly dumb question - did you tap save, like, with a touch screen? Or use a mouse?

Just a mouse.

@alabuzhev commented on GitHub (May 31, 2023): > you're logged in as "Desktop\Foo" but then run the Terminal as "Desktop\Bar" Yes. > A possibly dumb question - did you tap save, like, with a touch screen? Or use a mouse? Just a mouse.

claunia commented

@DHowett commented on GitHub (May 31, 2023):

Do you happen to have a dump or the ability to capture one? Alternatively, I'd settle for a Windows Error Reporting bucket ID (from the event viewer).

This is definitely us -- not XAML, probably -- but we don't know much beyond that from this stack.

@DHowett commented on GitHub (May 31, 2023): Do you happen to have a dump or the ability to capture one? Alternatively, I'd settle for a Windows Error Reporting bucket ID (from the event viewer). This is definitely us -- not XAML, probably -- but we don't know much beyond that from this stack.

claunia commented

@alabuzhev commented on GitHub (May 31, 2023):

Will this one do?
WindowsTerminal.dmp.zip

@alabuzhev commented on GitHub (May 31, 2023): Will this one do? [WindowsTerminal.dmp.zip](https://github.com/microsoft/terminal/files/11619218/WindowsTerminal.dmp.zip)

claunia commented

@DHowett commented on GitHub (Jun 1, 2023):

Close! I'm looking at it now, and as a minidump it doesn't have a bunch of the more helpful stuff.

Still, though, it does have this...

$$ At this frame
03 000000e4`601fc540 00007ff9`198a24ec     Windows_UI_Xaml!DirectUI::ErrorHelper::ReportUnhandledError+0xf1 [onecoreuap\windows\dxaml\xcp\dxaml\lib\errorhelper.cpp @ 502] 

$$ ?? errorInfo
class xxx::ErrorInfo
   +0x000 m_spErrorInfo    : ComPtr<IErrorInfo>
   +0x008 m_ErrorInfoInitialized : 0n1
   +0x010 m_spRestrictedErrorInfo : ComPtr<IRestrictedErrorInfo>
   +0x018 m_RestrictedErrorInfoInitialized : 0n1
   +0x020 m_description    : xxx
   +0x028 m_restrictedDescription : xxx
   +0x030 m_ErrorCode      : d0070005
   +0x034 m_ErrorDetailsInitialized : 0n1

Most of the members that are heap allocated--strings, etc--are missing... but that error code is shouting ACCESSDENIED at me.

Can you share the output of icacls actual\path\to\settings.json for the instance of Terminal that is failing like this?

@DHowett commented on GitHub (Jun 1, 2023): Close! I'm looking at it now, and as a minidump it doesn't have a bunch of the more helpful stuff. Still, though, it does have this... ``` $$ At this frame 03 000000e4`601fc540 00007ff9`198a24ec Windows_UI_Xaml!DirectUI::ErrorHelper::ReportUnhandledError+0xf1 [onecoreuap\windows\dxaml\xcp\dxaml\lib\errorhelper.cpp @ 502] $$ ?? errorInfo class xxx::ErrorInfo +0x000 m_spErrorInfo : ComPtr<IErrorInfo> +0x008 m_ErrorInfoInitialized : 0n1 +0x010 m_spRestrictedErrorInfo : ComPtr<IRestrictedErrorInfo> +0x018 m_RestrictedErrorInfoInitialized : 0n1 +0x020 m_description : xxx +0x028 m_restrictedDescription : xxx +0x030 m_ErrorCode : d0070005 +0x034 m_ErrorDetailsInitialized : 0n1 ``` Most of the members that are heap allocated--strings, etc--are missing... but that error code is shouting `ACCESSDENIED` at me. Can you share the output of `icacls actual\path\to\settings.json` for the instance of Terminal that is failing like this?

claunia commented

@alabuzhev commented on GitHub (Jun 1, 2023):

I can record a full dump, just wasn't sure github can handle a ~60MB zip attachment.

The output is:

NT AUTHORITY\SYSTEM:(I)(F)
BUILTIN\Administrators:(I)(F)
AA51M\Alex:(I)(F)

@alabuzhev commented on GitHub (Jun 1, 2023): I can record a full dump, just wasn't sure github can handle a ~60MB zip attachment. The output is: ``` NT AUTHORITY\SYSTEM:(I)(F) BUILTIN\Administrators:(I)(F) AA51M\Alex:(I)(F) ```

claunia commented

@DHowett commented on GitHub (Jun 1, 2023):

Interesting. That set of ACLs should work fine. I guess I'd be interested in the enclosing folder as well (since we try to do an atomic write!)

GitHub should be able to handle that size attachment, but if not my e-mail address is on my GitHub profile! 😄

@DHowett commented on GitHub (Jun 1, 2023): _Interesting_. That set of ACLs should work fine. I guess I'd be interested in the enclosing folder as well (since we try to do an atomic write!) GitHub should be able to handle that size attachment, but if not my e-mail address is on my GitHub profile! :smile:

claunia commented

@alabuzhev commented on GitHub (Jun 1, 2023):

the enclosing folder as well

NT AUTHORITY\SYSTEM:(I)(OI)(CI)(F)
BUILTIN\Administrators:(I)(OI)(CI)(F)
AA51M\Alex:(I)(OI)(CI)(F)

GitHub should be able to handle that size attachment

It says "25 MB are allowed".
Can you access this: https://file.io/LpFyCGYknwqG?
If not I'll try e-mail.

@alabuzhev commented on GitHub (Jun 1, 2023): > the enclosing folder as well ``` NT AUTHORITY\SYSTEM:(I)(OI)(CI)(F) BUILTIN\Administrators:(I)(OI)(CI)(F) AA51M\Alex:(I)(OI)(CI)(F) ``` > GitHub should be able to handle that size attachment It says "25 MB are allowed". Can you access this: https://file.io/LpFyCGYknwqG? If not I'll try e-mail.

claunia commented

@alabuzhev commented on GitHub (Jun 1, 2023):

Most of the members that are heap allocated--strings, etc--are missing... but that error code is shouting ACCESSDENIED at me.

Indeed, d0070005 looks like ERROR_ACCESS_DENIED, packed into NTSTATUS with Facility = FACILITY_NTWIN32, Severity=Error, packed again into HRESULT.
The string isn't that exciting though: "The text associated with this error code could not be found..."

@alabuzhev commented on GitHub (Jun 1, 2023): > Most of the members that are heap allocated--strings, etc--are missing... but that error code is shouting ACCESSDENIED at me. Indeed, d0070005 looks like ERROR_ACCESS_DENIED, packed into NTSTATUS with Facility = FACILITY_NTWIN32, Severity=Error, packed again into HRESULT. The string isn't that exciting though: "The text associated with this error code could not be found..."

claunia commented

@alabuzhev commented on GitHub (Jun 1, 2023):

Ugh, found it.
It was trying to create a registry key, HKEY_CURRENT_USER\Console\%%Startup, and creating subkeys under "Console" was denied (for unrelated reasons).

@alabuzhev commented on GitHub (Jun 1, 2023): Ugh, found it. It was trying to create a registry key, `HKEY_CURRENT_USER\Console\%%Startup`, and creating subkeys under "Console" was denied (for unrelated reasons).

claunia commented

@DHowett commented on GitHub (Jun 1, 2023):

OHHHH booy. Okay. Are those reasons that would impact, say, a user who has never used Terminal on their machine? Or are those "@alabuzhev reasons" ;P

@DHowett commented on GitHub (Jun 1, 2023): OHHHH booy. Okay. Are those reasons that would impact, say, a user who has never used Terminal on their machine? Or are those "@alabuzhev reasons" ;P

claunia commented

@alabuzhev commented on GitHub (Jun 2, 2023):

"I can explain!"

In previous Windows versions, when you open console properties and change something, it was always asking you "Apply properties to current window only" / "Save properties for future windows with same title":

It was quite useful: if you need to experiment with console settings (and if you develop non-trivial console apps, affected by those settings - you do), you just choose "current window only", and all your silly experiments disappear when you close this console window.

But in newer Windows versions that useful confirmation was removed for unknown reason and the properties are now always quietly saved as if "save for future windows with the same title" was chosen, meaning that all your silly experiments are now persisted immediately and you have to undo them later manually like an idiot. It is no longer possible to make changes for the current session only.

Denying "Create subkey" for "Console" is a quick and dirty workaround to say "no, please don't save anything into registry, thank you very much".

By the way, was removing that confirmation a deliberate design decision?
Any chance to get it back?

@alabuzhev commented on GitHub (Jun 2, 2023): "I can explain!" In previous Windows versions, when you open console properties and change something, it was always asking you "Apply properties to current window only" / "Save properties for future windows with same title": ![image](https://github.com/microsoft/terminal/assets/11453922/b843f31d-b66a-4d97-8862-490d4768fa8b) It was quite useful: if you need to experiment with console settings (and if you develop non-trivial console apps, affected by those settings - you do), you just choose "current window only", and all your silly experiments disappear when you close this console window. But in newer Windows versions that useful confirmation was removed for unknown reason and the properties are now always quietly saved as if "save for future windows with the same title" was chosen, meaning that all your silly experiments are now persisted immediately and you have to undo them later manually like an idiot. It is no longer possible to make changes *for the current session only*. Denying "Create subkey" for "Console" is a quick and dirty workaround to say "no, please don't save anything into registry, thank you very much". By the way, was removing that confirmation a deliberate design decision? Any chance to get it back?

claunia commented

@alabuzhev commented on GitHub (Jun 2, 2023):

Are those reasons that would impact, say, a user who has never used Terminal on their machine?

Maybe, who knows. I shared this workaround with a few people, someone else could have discovered it independently.

@alabuzhev commented on GitHub (Jun 2, 2023): > Are those reasons that would impact, say, a user who has never used Terminal on their machine? Maybe, who knows. I shared this workaround with a few people, someone else could have discovered it independently.

claunia commented

@DHowett commented on GitHub (Jun 5, 2023):

By the way, was removing that confirmation a deliberate design decision?
Any chance to get it back?

That is an excellent question!

I can't get at the Vista source depot right now, but I can see that it disappeared sometime between Windows Server 2003 and Windows 7. My tooling isn't working quite right, so I can't find the exact changeset that removed it. If I get it working again (🙄) I'll dig further.

@DHowett commented on GitHub (Jun 5, 2023): > By the way, was removing that confirmation a deliberate design decision? > Any chance to get it back? That is an excellent question! I can't get at the Vista source depot right now, but I _can_ see that it disappeared sometime between Windows Server 2003 and Windows 7. My tooling isn't working quite right, so I can't find the exact changeset that removed it. If I get it working again (🙄) I'll dig further.

claunia commented

@DHowett commented on GitHub (Jul 19, 2023):

I'm taking Attention off this and committing it to the triaged list. We should, at the very least, not crash.

@DHowett commented on GitHub (Jul 19, 2023): I'm taking `Attention` off this and committing it to the triaged list. We should, at the very least, not crash.

claunia commented

@alabuzhev commented on GitHub (Sep 5, 2023):

@DHowett thanks.
Any chance of some attention to the "root cause" of this (no way to have in-memory-only settings without committing them to registry/shortcut after Vista or 7)?

@alabuzhev commented on GitHub (Sep 5, 2023): @DHowett thanks. Any chance of some attention to the "root cause" of this (no way to have in-memory-only settings without committing them to registry/shortcut after Vista or 7)?

claunia commented

@DHowett commented on GitHub (Sep 6, 2023):

Sure! It'd be better coming from you--would you mind filing a feature request for it?
@lhecker and I were talking about how the preexisting (pre-Vista) model didn't make the most sense, but that it's something we would like to have on the backlog for a rainy day. 😄

@DHowett commented on GitHub (Sep 6, 2023): Sure! It'd be better coming from you--would you mind filing a feature request for it? @lhecker and I were talking about how the preexisting (pre-Vista) model didn't make the most sense, but that it's something we would like to have on the backlog for a rainy day. :smile:

claunia commented