Add fix for nested (Debug-Runspace) debugging on non-Windows platforms and re-enable Windows identity impersonation flow#8451
Add fix for nested (Debug-Runspace) debugging on non-Windows platforms and re-enable Windows identity impersonation flow#8451daxian-dbw merged 8 commits intoPowerShell:masterfrom PaulHigin:fix-unix-ipc-debugging
Conversation
daxian-dbw
left a comment
There was a problem hiding this comment.
@TravisEz13 @PaulHigin We ran into an issue before with WindowsIdentity.RunImpersonated(safeAccessTokenHandle, action) on Windows when porting powershell to CoreCLR, and it caused powershell to crash. We (us and .NET Core team) never figured out what was the root cause. I don't know if that issue is gone with the latest .NET Core. I will forward you the email thread on this.
src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs
Show resolved
Hide resolved
…t RunImpersonated.
|
Looking through the code I realized there were many more places in the engine where Windows identity impersonation flow was disabled. This is needed to support rare but important Windows scenarios. I submitted a commit that re-enables these using the new dotNet RunImpersonated API. I will also update the title to reflect these new changes. |
|
@daxian-dbw All tests have passed. Can this be merged? |
PR Summary
Nested runspace debugging is now supported on non-Windows platforms after Enter-PSHostProcess was added. However, nested debugging was broken because the Windows impersonation code portion was not fully ifdefed out.
This change removes Windows only impersonation code for cross platform builds. It also updates the Windows impersonation flow code to work with dotNet Core.
Repro:
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests