Skip to content

Update UseNewEnvironment parameter behavior of Start-Process cmdlet on Windows #10830

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.

Sign up for GitHub

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

Merged
adityapatwardhan merged 1 commit into from
Apr 18, 2020
Merged

Update UseNewEnvironment parameter behavior of Start-Process cmdlet on Windows #10830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 64 additions & 21 deletions src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2348,22 +2348,8 @@ private static byte[] ConvertEnvVarsToByteArray(StringDictionary sd)
return bytes;
}

/// <summary>
/// This method will be used on all windows platforms, both full desktop and headless SKUs.
/// </summary>
private Process StartWithCreateProcess(ProcessStartInfo startinfo)
private void SetStartupInfo(ProcessStartInfo startinfo, ref ProcessNativeMethods.STARTUPINFO lpStartupInfo, ref int creationFlags)
{
ProcessNativeMethods.STARTUPINFO lpStartupInfo = new ProcessNativeMethods.STARTUPINFO();
SafeNativeMethods.PROCESS_INFORMATION lpProcessInformation = new SafeNativeMethods.PROCESS_INFORMATION();
int error = 0;
GCHandle pinnedEnvironmentBlock = new GCHandle();
string message = string.Empty;

// building the cmdline with the file name given and it's arguments
StringBuilder cmdLine = BuildCommandLine(startinfo.FileName, startinfo.Arguments);

try
{
// RedirectionStandardInput
if (_redirectstandardinput != null)
{
Expand All @@ -2375,6 +2361,7 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
{
lpStartupInfo.hStdInput = new SafeFileHandle(ProcessNativeMethods.GetStdHandle(-10), false);
}

// RedirectionStandardOutput
if (_redirectstandardoutput != null)
{
Expand All @@ -2386,6 +2373,7 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
{
lpStartupInfo.hStdOutput = new SafeFileHandle(ProcessNativeMethods.GetStdHandle(-11), false);
}

// RedirectionStandardError
if (_redirectstandarderror != null)
{
Expand All @@ -2397,11 +2385,10 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
{
lpStartupInfo.hStdError = new SafeFileHandle(ProcessNativeMethods.GetStdHandle(-12), false);
}

// STARTF_USESTDHANDLES
lpStartupInfo.dwFlags = 0x100;

int creationFlags = 0;

if (startinfo.CreateNoWindow)
{
// No new window: Inherit the parent process's console window
Expand All @@ -2411,6 +2398,7 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
{
// CREATE_NEW_CONSOLE
creationFlags |= 0x00000010;

// STARTF_USESHOWWINDOW
lpStartupInfo.dwFlags |= 0x00000001;

Expand Down Expand Up @@ -2438,15 +2426,41 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)

// Create the new process suspended so we have a chance to get a corresponding Process object in case it terminates quickly.
creationFlags |= 0x00000004;
}

IntPtr AddressOfEnvironmentBlock = IntPtr.Zero;
var environmentVars = startinfo.EnvironmentVariables;
if (environmentVars != null)
/// <summary>
/// This method will be used on all windows platforms, both full desktop and headless SKUs.
/// </summary>
private Process StartWithCreateProcess(ProcessStartInfo startinfo)
{
ProcessNativeMethods.STARTUPINFO lpStartupInfo = new ProcessNativeMethods.STARTUPINFO();
SafeNativeMethods.PROCESS_INFORMATION lpProcessInformation = new SafeNativeMethods.PROCESS_INFORMATION();
int error = 0;
GCHandle pinnedEnvironmentBlock = new GCHandle();
IntPtr AddressOfEnvironmentBlock = IntPtr.Zero;
string message = string.Empty;

// building the cmdline with the file name given and it's arguments
StringBuilder cmdLine = BuildCommandLine(startinfo.FileName, startinfo.Arguments);

try
{
int creationFlags = 0;

SetStartupInfo(startinfo, ref lpStartupInfo, ref creationFlags);

// We follow the logic:
// - Ignore `UseNewEnvironment` when we run a process as another user.
// Setting initial environment variables makes sense only for current user.
// - Set environment variables if they present in ProcessStartupInfo.
if (!UseNewEnvironment)
{
if (this.UseNewEnvironment)
var environmentVars = startinfo.EnvironmentVariables;
if (environmentVars != null)
{
// All Windows Operating Systems that we support are Windows NT systems, so we use Unicode for environment.
creationFlags |= 0x400;

pinnedEnvironmentBlock = GCHandle.Alloc(ConvertEnvVarsToByteArray(environmentVars), GCHandleType.Pinned);
AddressOfEnvironmentBlock = pinnedEnvironmentBlock.AddrOfPinnedObject();
}
Expand All @@ -2456,6 +2470,7 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)

if (_credential != null)
{
// Run process as another user.
ProcessNativeMethods.LogonFlags logonFlags = 0;
if (startinfo.LoadUserProfile)
{
Expand Down Expand Up @@ -2504,6 +2519,22 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
}
}

// Run process as current user.
if (UseNewEnvironment)
{
// All Windows Operating Systems that we support are Windows NT systems, so we use Unicode for environment.
creationFlags |= 0x400;

IntPtr token = WindowsIdentity.GetCurrent().Token;
if (!ProcessNativeMethods.CreateEnvironmentBlock(out AddressOfEnvironmentBlock, token, false))
{
Win32Exception win32ex = new Win32Exception(error);
message = StringUtil.Format(ProcessResources.InvalidStartProcess, win32ex.Message);
var errorRecord = new ErrorRecord(new InvalidOperationException(message), "InvalidOperationException", ErrorCategory.InvalidOperation, null);
ThrowTerminatingError(errorRecord);
}
}

ProcessNativeMethods.SECURITY_ATTRIBUTES lpProcessAttributes = new ProcessNativeMethods.SECURITY_ATTRIBUTES();
ProcessNativeMethods.SECURITY_ATTRIBUTES lpThreadAttributes = new ProcessNativeMethods.SECURITY_ATTRIBUTES();
flag = ProcessNativeMethods.CreateProcess(null, cmdLine, lpProcessAttributes, lpThreadAttributes, true, creationFlags, AddressOfEnvironmentBlock, startinfo.WorkingDirectory, lpStartupInfo, lpProcessInformation);
Expand Down Expand Up @@ -2531,6 +2562,10 @@ private Process StartWithCreateProcess(ProcessStartInfo startinfo)
{
pinnedEnvironmentBlock.Free();
}
else
{
ProcessNativeMethods.DestroyEnvironmentBlock(AddressOfEnvironmentBlock);
}

lpStartupInfo.Dispose();
lpProcessInformation.Dispose();
Expand Down Expand Up @@ -2720,6 +2755,14 @@ public static extern FileNakedHandle CreateFileW(
System.IntPtr hTemplateFile
);

[DllImport("userenv.dll", CharSet = CharSet.Unicode, SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static extern bool CreateEnvironmentBlock(out IntPtr lpEnvironment, IntPtr hToken, bool bInherit);

[DllImport("userenv.dll", CharSet = CharSet.Unicode, SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static extern bool DestroyEnvironmentBlock(IntPtr lpEnvironment);

[Flags]
internal enum LogonFlags
{
Expand Down
60 changes: 44 additions & 16 deletions test/powershell/Modules/Microsoft.PowerShell.Management/Start-Process.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

Describe "Start-Process" -Tag "Feature","RequireAdminOnWindows" {

BeforeAll {
Expand All @@ -19,75 +20,75 @@ Describe "Start-Process" -Tag "Feature","RequireAdminOnWindows" {
$pingParam = "-n 2 localhost"
}
elseif ($IsLinux -Or $IsMacOS) {
$pingParam = "-c 2 localhost"
$pingParam = "-c 2 localhost"
}
}

# Note that ProcessName may still be `powershell` due to dotnet/corefx#5378
# This has been fixed on Linux, but not on macOS

It "Should process arguments without error" {
$process = Start-Process ping -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
$process = Start-Process ping -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs

$process.Length | Should -Be 1
$process.Id | Should -BeGreaterThan 1
# $process.ProcessName | Should -Be "ping"
}

It "Should work correctly when used with full path name" {
$process = Start-Process $pingCommand -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
$process = Start-Process $pingCommand -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs

$process.Length | Should -Be 1
$process.Id | Should -BeGreaterThan 1
# $process.ProcessName | Should -Be "ping"
}

It "Should invoke correct path when used with FilePath argument" {
$process = Start-Process -FilePath $pingCommand -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
$process = Start-Process -FilePath $pingCommand -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs

$process.Length | Should -Be 1
$process.Id | Should -BeGreaterThan 1
# $process.ProcessName | Should -Be "ping"
}

It "Should invoke correct path when used with Path alias argument" {
$process = Start-Process -Path $pingCommand -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
$process = Start-Process -Path $pingCommand -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs

$process.Length | Should -Be 1
$process.Id | Should -BeGreaterThan 1
$process.Length | Should -Be 1
$process.Id | Should -BeGreaterThan 1
}

It "Should wait for command completion if used with Wait argument" {
$process = Start-Process ping -ArgumentList $pingParam -Wait -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
$process = Start-Process ping -ArgumentList $pingParam -Wait -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
}

It "Should work correctly with WorkingDirectory argument" {
$process = Start-Process ping -WorkingDirectory $pingDirectory -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
$process = Start-Process ping -WorkingDirectory $pingDirectory -ArgumentList $pingParam -PassThru -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs

$process.Length | Should -Be 1
$process.Id | Should -BeGreaterThan 1
# $process.ProcessName | Should -Be "ping"
}

It "Should handle stderr redirection without error" {
$process = Start-Process ping -ArgumentList $pingParam -PassThru -RedirectStandardError $tempFile -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs
$process = Start-Process ping -ArgumentList $pingParam -PassThru -RedirectStandardError $tempFile -RedirectStandardOutput "$TESTDRIVE/output" @extraArgs

$process.Length | Should -Be 1
$process.Id | Should -BeGreaterThan 1
# $process.ProcessName | Should -Be "ping"
}

It "Should handle stdout redirection without error" {
$process = Start-Process ping -ArgumentList $pingParam -Wait -RedirectStandardOutput $tempFile @extraArgs
$dirEntry = get-childitem $tempFile
$dirEntry.Length | Should -BeGreaterThan 0
$process = Start-Process ping -ArgumentList $pingParam -Wait -RedirectStandardOutput $tempFile @extraArgs
$dirEntry = get-childitem $tempFile
$dirEntry.Length | Should -BeGreaterThan 0
}

# Marking this test 'pending' to unblock daily builds. Filed issue : https://github.com/PowerShell/PowerShell/issues/2396
It "Should handle stdin redirection without error" -Pending {
$process = Start-Process sort -Wait -RedirectStandardOutput $tempFile -RedirectStandardInput $assetsFile @extraArgs
$dirEntry = get-childitem $tempFile
$dirEntry.Length | Should -BeGreaterThan 0
$process = Start-Process sort -Wait -RedirectStandardOutput $tempFile -RedirectStandardInput $assetsFile @extraArgs
$dirEntry = get-childitem $tempFile
$dirEntry.Length | Should -BeGreaterThan 0
}

## -Verb is supported in PowerShell on Windows full desktop.
Expand Down Expand Up @@ -169,3 +170,30 @@ Describe "Start-Process tests requiring admin" -Tags "Feature","RequireAdminOnWi
Get-Content $testdrive\foo.txt | Should -BeExactly $fooFile
}
}

Describe "Start-Process" -Tags "Feature" {

It "UseNewEnvironment parameter should reset environment variables for child process" {

$PWSH = (Get-Process -Id $PID).MainModule.FileName
$outputFile = Join-Path -Path $TestDrive -ChildPath output.txt

$env:TestEnvVariable | Should -BeNullOrEmpty

$env:TestEnvVariable = 1
$userName = $env:USERNAME

try {
Start-Process $PWSH -ArgumentList '-NoProfile','-Command Write-Output \"$($env:TestEnvVariable);$($env:USERNAME)\"' -RedirectStandardOutput $outputFile -Wait
Get-Content -LiteralPath $outputFile | Should -BeExactly "1;$userName"

# Check that:
# 1. Environment variables is resetted (TestEnvVariable is removed)
# 2. Environment variables comes from current user profile
Start-Process $PWSH -ArgumentList '-NoProfile','-Command Write-Output \"$($env:TestEnvVariable);$($env:USERNAME)\"' -RedirectStandardOutput $outputFile -Wait -UseNewEnvironment
Get-Content -LiteralPath $outputFile | Should -BeExactly ";$userName"
} finally {
$env:TestEnvVariable = $null
}
}
}