From 44671c64f47afa203b345255c764cdfd49774fe6 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Mon, 18 May 2026 15:54:21 -0400 Subject: [PATCH 1/4] fix user path regression --- contrib/win32/win32compat/w32fd.c | 162 +++++++++++++----- regress/pesterTests/UserEnvironment.Tests.ps1 | 118 +++++++++++++ 2 files changed, 236 insertions(+), 44 deletions(-) create mode 100644 regress/pesterTests/UserEnvironment.Tests.ps1 diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index b4c436864dc4..2420ef179a0a 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -1055,43 +1055,88 @@ int fork() } char * build_commandline_string(const char* cmd, char *const argv[], BOOLEAN prepend_module_path); -wchar_t* -get_username_from_token(HANDLE as_user) +static BOOL +is_sshd_service_token(HANDLE token) { - wchar_t* user_name = NULL; - SID_NAME_USE usage; + BOOL is_sshd = FALSE; DWORD count = 0; - GetTokenInformation(as_user, TokenUser, NULL, 0, &count); - if (count) { - void* buffer = malloc(count); - if (buffer) { - if (GetTokenInformation(as_user, TokenUser, buffer, count, &count)) { - TOKEN_USER* owner = (TOKEN_USER*)buffer; - DWORD name_length = 0; - DWORD domain_length = 0; - LookupAccountSidW(NULL, owner->User.Sid, NULL, &name_length, NULL, &domain_length, &usage); /* Figure out the length of the name. */ - if (name_length) { - wchar_t* domain_name = malloc(domain_length * sizeof(wchar_t)); - if (domain_name) { - user_name = malloc(name_length * sizeof(wchar_t)); - if (user_name) { - memset(user_name, 0, name_length * sizeof(wchar_t)); - memset(domain_name, 0, domain_length * sizeof(wchar_t)); - BOOL success = LookupAccountSidW(NULL, owner->User.Sid, user_name, &name_length, domain_name, &domain_length, &usage); - if (!success) /* Silently return an empty string if unsuccessful. */ - { - free(user_name); - user_name = NULL; - } - } - free(domain_name); - } - } + GetTokenInformation(token, TokenUser, NULL, 0, &count); + if (!count) + return FALSE; + void* buffer = malloc(count); + if (!buffer) + return FALSE; + if (GetTokenInformation(token, TokenUser, buffer, count, &count)) { + TOKEN_USER* user = (TOKEN_USER*)buffer; + SID_NAME_USE usage; + DWORD name_len = 0, domain_len = 0; + LookupAccountSidW(NULL, user->User.Sid, NULL, &name_len, NULL, &domain_len, &usage); + if (name_len && domain_len) { + wchar_t* name = malloc(name_len * sizeof(wchar_t)); + wchar_t* domain = malloc(domain_len * sizeof(wchar_t)); + if (name && domain && + LookupAccountSidW(NULL, user->User.Sid, name, &name_len, domain, &domain_len, &usage)) { + is_sshd = (_wcsicmp(name, L"sshd") == 0 && _wcsicmp(domain, L"NT SERVICE") == 0); } - free(buffer); + if (name) + free(name); + if (domain) + free(domain); + } + } + free(buffer); + return is_sshd; +} + +static wchar_t* +override_path_in_env_block(const wchar_t* src, const wchar_t* merged_path) +{ + if (!src || !merged_path) + return NULL; + + const size_t prefix_len = 5; /* L"PATH=" */ + size_t merged_path_len = wcslen(merged_path); + size_t path_entry_chars = prefix_len + merged_path_len + 1; /* include trailing NUL */ + + /* Measure source block length (in wchars) including the final empty string. */ + const wchar_t* p = src; + while (*p) + p += wcslen(p) + 1; + size_t src_chars = (size_t)(p - src) + 1; /* +1 for the terminating empty string */ + + /* Worst case: original block plus one extra PATH entry. */ + size_t out_capacity = src_chars + path_entry_chars; + wchar_t* out = (wchar_t*)malloc(out_capacity * sizeof(wchar_t)); + if (!out) + return NULL; + + wchar_t* w = out; + BOOL replaced = FALSE; + const wchar_t* cur = src; + while (*cur) { + size_t entry_chars = wcslen(cur); /* excludes NUL */ + if (!replaced && entry_chars >= prefix_len && _wcsnicmp(cur, L"PATH=", prefix_len) == 0) { + memcpy(w, L"PATH=", prefix_len * sizeof(wchar_t)); + w += prefix_len; + memcpy(w, merged_path, merged_path_len * sizeof(wchar_t)); + w += merged_path_len; + *w++ = L'\0'; + replaced = TRUE; + } else { + memcpy(w, cur, (entry_chars + 1) * sizeof(wchar_t)); + w += entry_chars + 1; } + cur += entry_chars + 1; + } + if (!replaced) { + memcpy(w, L"PATH=", prefix_len * sizeof(wchar_t)); + w += prefix_len; + memcpy(w, merged_path, merged_path_len * sizeof(wchar_t)); + w += merged_path_len; + *w++ = L'\0'; } - return user_name; + *w = L'\0'; /* final double-NUL terminator */ + return out; } /* @@ -1106,10 +1151,11 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, { PROCESS_INFORMATION pi; STARTUPINFOW si; - BOOL b; + BOOL b = FALSE; char *cmdline; wchar_t * cmdline_utf16 = NULL; int ret = -1; + if ((cmdline = build_commandline_string(cmd, argv, prepend_module_path)) == NULL) { errno = ENOMEM; goto cleanup; @@ -1125,7 +1171,7 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, si.hStdOutput = out; si.hStdError = err; si.dwFlags = STARTF_USESTDHANDLES; - + if (strstr(cmd, "sshd-session.exe") || strstr(cmd, "sshd-auth.exe")) { flags |= DETACHED_PROCESS; } @@ -1146,20 +1192,48 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, if (as_user) { debug3("spawning %ls as user", t); LPVOID lpEnvironment = NULL; - wchar_t* as_user_name = get_username_from_token(as_user); - if (as_user_name) { - if (wcsncmp(L"sshd", as_user_name, wcslen(L"sshd")) != 0) { /* Ignore any names that begin with the service name `sshd`. */ - b = CreateEnvironmentBlock(&lpEnvironment, as_user, TRUE); /* Load a user environment block inheriting the current context, thereby passing session state. */ + wchar_t* merged_env = NULL; + if (!is_sshd_service_token(as_user)) { + /* Load the user's environment block (HKCU vars, USERPROFILE, etc.), + * inheriting the current context so session state set in + * sshd-session is preserved. Skipped for the NT SERVICE\sshd + * virtual account (sshd worker chain re-spawning itself). */ + CreateEnvironmentBlock(&lpEnvironment, as_user, TRUE); + } + + if (lpEnvironment) { + /* Restore the merged System+User PATH that setup_session_user_vars + * computed into this process's PATH; CreateEnvironmentBlock + * overlays HKCU PATH on top of it, so we must override the block. */ + DWORD merged_path_chars = GetEnvironmentVariableW(L"PATH", NULL, 0); + if (merged_path_chars > 0) { + wchar_t* merged_path = (wchar_t*)malloc(merged_path_chars * sizeof(wchar_t)); + if (merged_path) { + if (GetEnvironmentVariableW(L"PATH", merged_path, merged_path_chars) > 0) + merged_env = override_path_in_env_block((const wchar_t*)lpEnvironment, merged_path); + free(merged_path); + } } - free(as_user_name); } - if (lpEnvironment) { /* Pass the user environment block to the new process. */ - b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags | CREATE_UNICODE_ENVIRONMENT, lpEnvironment, NULL, &si, &pi); - DestroyEnvironmentBlock(lpEnvironment); + + if (merged_env) { + b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, + flags | CREATE_UNICODE_ENVIRONMENT, + merged_env, NULL, &si, &pi); + free(merged_env); + } + else if (lpEnvironment) { + b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, + flags | CREATE_UNICODE_ENVIRONMENT, + lpEnvironment, NULL, &si, &pi); } - else { /* Pass the current context's environment block to the new process. */ - b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags, NULL, NULL, &si, &pi); + else { + /* Fallback: pass the current context's environment block. */ + b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, + flags, NULL, NULL, &si, &pi); } + if (lpEnvironment) + DestroyEnvironmentBlock(lpEnvironment); } else { debug3("spawning %ls as subprocess", t); diff --git a/regress/pesterTests/UserEnvironment.Tests.ps1 b/regress/pesterTests/UserEnvironment.Tests.ps1 new file mode 100644 index 000000000000..3358e5630fce --- /dev/null +++ b/regress/pesterTests/UserEnvironment.Tests.ps1 @@ -0,0 +1,118 @@ +If ($PSVersiontable.PSVersion.Major -le 2) {$PSScriptRoot = Split-Path -Parent $MyInvocation.MyCommand.Path} +Import-Module $PSScriptRoot\CommonUtils.psm1 -Force + +$tC = 1 +$tI = 0 +$suite = "userenvironment" + +Describe "E2E scenarios for user environment block" -Tags "CI" { + BeforeAll { + if ($OpenSSHTestInfo -eq $null) { + Throw "`$OpenSSHTestInfo is null. Please run Set-OpenSSHTestEnvironment to set test environments." + } + + $server = $OpenSSHTestInfo["Target"] + $port = $OpenSSHTestInfo["Port"] + $ssouser = $OpenSSHTestInfo["SSOUser"] + + $testDir = Join-Path $OpenSSHTestInfo["TestDataPath"] $suite + if (-not (Test-Path $testDir)) { + $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue + } + + # Helper: run a one-liner inside an SSH session as the SSO user using PowerShell. + function Invoke-RemotePS { + param([Parameter(Mandatory=$true)][string] $Script) + $encoded = [Convert]::ToBase64String([System.Text.Encoding]::Unicode.GetBytes($Script)) + $out = ssh -p $port -o "StrictHostKeyChecking=no" -o "UserKnownHostsFile=$testDir\known_hosts" ` + "$ssouser@$server" powershell -NoProfile -NonInteractive -EncodedCommand $encoded 2>$null + if ($out -is [array]) { return ($out -join "`n").Trim() } + return ($out | Out-String).Trim() + } + } + + AfterEach { $tI++ } + + Context "$tC - User environment variables" { + BeforeAll { $tI = 1 } + AfterAll { $tC++ } + + It "$tC.$tI - USERNAME matches the connecting user" { + $shortName = ($ssouser -split '\\')[-1] + $remote = Invoke-RemotePS '$env:USERNAME' + $remote | Should Be $shortName + } + + It "$tC.$tI - USERPROFILE points to the connecting user's profile" { + $remote = Invoke-RemotePS '$env:USERPROFILE' + $remote | Should Not BeNullOrEmpty + # Reject the LocalSystem / sshd-session fallback profile. + $remote | Should Not Match 'system32\\config\\systemprofile' + ($remote -split '\\')[-1] | Should Be (($ssouser -split '\\')[-1]) + } + + It "$tC.$tI - HOMEDRIVE and HOMEPATH are populated" { + $remote = Invoke-RemotePS '"{0}|{1}" -f $env:HOMEDRIVE, $env:HOMEPATH' + $parts = $remote -split '\|' + $parts[0] | Should Match '^[A-Za-z]:$' + $parts[1] | Should Not BeNullOrEmpty + } + + It "$tC.$tI - APPDATA and LOCALAPPDATA are populated and per-user" { + $remote = Invoke-RemotePS '"{0}|{1}" -f $env:APPDATA, $env:LOCALAPPDATA' + $parts = $remote -split '\|' + $parts[0] | Should Match 'AppData\\Roaming$' + $parts[1] | Should Match 'AppData\\Local$' + $parts[0] | Should Not Match 'system32\\config\\systemprofile' + $parts[1] | Should Not Match 'system32\\config\\systemprofile' + } + + It "$tC.$tI - USERDOMAIN is populated and is not WORKGROUP" { + $remote = Invoke-RemotePS '$env:USERDOMAIN' + $remote | Should Not BeNullOrEmpty + $remote | Should Not Be 'WORKGROUP' + } + } + + Context "$tC - PATH is the merged System + User PATH (Phase 1 fix)" { + BeforeAll { + $tI = 1 + # Write a unique marker into the SSO user's HKCU\Environment\Path + # from within an SSH session (we are not the user locally, so we + # cannot edit their HKCU directly without loading the hive). + $script:pathMarker = "C:\sshtestmarker_$([guid]::NewGuid().ToString('N'))" + $setScript = @" +`$existing = (Get-ItemProperty -Path 'HKCU:\Environment' -Name Path -ErrorAction SilentlyContinue).Path +if (-not `$existing) { `$existing = '' } +if (-not (`$existing -split ';' -contains '$($script:pathMarker)')) { + `$new = if (`$existing) { `$existing.TrimEnd(';') + ';$($script:pathMarker)' } else { '$($script:pathMarker)' } + Set-ItemProperty -Path 'HKCU:\Environment' -Name Path -Value `$new -Type ExpandString +} +"@ + Invoke-RemotePS $setScript | Out-Null + } + AfterAll { + $clearScript = @" +`$existing = (Get-ItemProperty -Path 'HKCU:\Environment' -Name Path -ErrorAction SilentlyContinue).Path +if (`$existing) { + `$new = ((`$existing -split ';') | Where-Object { `$_ -ne '$($script:pathMarker)' }) -join ';' + Set-ItemProperty -Path 'HKCU:\Environment' -Name Path -Value `$new -Type ExpandString +} +"@ + Invoke-RemotePS $clearScript | Out-Null + $tC++ + } + + It "$tC.$tI - PATH contains BOTH the system entry and the user marker" { + $remote = Invoke-RemotePS '$env:PATH' + $segments = $remote -split ';' + ($segments | Where-Object { $_ -match '[Ss]ystem32' }).Count | Should BeGreaterThan 0 + ($segments | Where-Object { $_ -eq $script:pathMarker }).Count | Should BeGreaterThan 0 + $sysIndex = ($segments | ForEach-Object { $_ }).IndexOf((($segments | Where-Object { $_ -match '[Ss]ystem32' }) | Select-Object -First 1)) + $markerIndex = ($segments | ForEach-Object { $_ }).IndexOf($script:pathMarker) + $sysIndex | Should BeGreaterThan -1 + $markerIndex | Should BeGreaterThan -1 + $sysIndex | Should BeLessThan $markerIndex + } + } +} From f7c23f7bc0226909113c5946caffcb01969cda6c Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 19 May 2026 14:48:34 -0400 Subject: [PATCH 2/4] add tests for user env vars --- contrib/win32/openssh/OpenSSHTestHelper.psm1 | 7 +- regress/pesterTests/UserEnvironment.Tests.ps1 | 133 ++++++++++-------- 2 files changed, 79 insertions(+), 61 deletions(-) diff --git a/contrib/win32/openssh/OpenSSHTestHelper.psm1 b/contrib/win32/openssh/OpenSSHTestHelper.psm1 index 84f3ce07f6d3..2fc90313bd3c 100644 --- a/contrib/win32/openssh/OpenSSHTestHelper.psm1 +++ b/contrib/win32/openssh/OpenSSHTestHelper.psm1 @@ -15,8 +15,9 @@ $PubKeyUser = "sshtest_pubkeyuser" $PasswdUser = "sshtest_passwduser" $AdminUser = "sshtest_adminuser" $NonAdminUser = "sshtest_nonadminuser" +$SshdUser = "sshd_user" $OpenSSHTestAccountsPassword = "Bulldog_123456" -$OpenSSHTestAccounts = $Script:SSOUser, $Script:PubKeyUser, $Script:PasswdUser, $Script:AdminUser, $Script:NonAdminUser +$OpenSSHTestAccounts = $Script:SSOUser, $Script:PubKeyUser, $Script:PasswdUser, $Script:AdminUser, $Script:NonAdminUser, $Script:SshdUser $SSHDTestSvcName = "sshdTestSvc" $Script:TestDataPath = "$env:SystemDrive\OpenSSHTests" @@ -69,6 +70,7 @@ function Set-OpenSSHTestEnvironment $Global:OpenSSHTestInfo.Add("PasswdUser", $PasswdUser) # test user to be used for password auth $Global:OpenSSHTestInfo.Add("AdminUser", $AdminUser) # test user to be used for admin logging tests $Global:OpenSSHTestInfo.Add("NonAdminUser", $NonAdminUser) # test user to be used for non-admin logging tests + $Global:OpenSSHTestInfo.Add("SshdUser", $SshdUser) # non-admin local user used by UserEnvironment tests $Global:OpenSSHTestInfo.Add("TestAccountPW", $OpenSSHTestAccountsPassword) # common password for all test accounts $Global:OpenSSHTestInfo.Add("DebugMode", $DebugMode.IsPresent) # run openssh E2E in debug mode $Global:OpenSSHTestInfo.Add("DelayTime", 3) # delay between stoppig sshd service and trying to access log files @@ -225,6 +227,9 @@ WARNING: Following changes will be made to OpenSSH configuration $NonAdminUserProfile = Get-LocalUserProfile -User $NonAdminUser $Global:OpenSSHTestInfo.Add("NonAdminUserProfile", $NonAdminUserProfile) + $SshdUserProfile = Get-LocalUserProfile -User $SshdUser + $Global:OpenSSHTestInfo.Add("SshdUserProfile", $SshdUserProfile) + #make $AdminUser admin net localgroup Administrators $AdminUser /add diff --git a/regress/pesterTests/UserEnvironment.Tests.ps1 b/regress/pesterTests/UserEnvironment.Tests.ps1 index 3358e5630fce..3708ed432b7d 100644 --- a/regress/pesterTests/UserEnvironment.Tests.ps1 +++ b/regress/pesterTests/UserEnvironment.Tests.ps1 @@ -1,5 +1,6 @@ If ($PSVersiontable.PSVersion.Major -le 2) {$PSScriptRoot = Split-Path -Parent $MyInvocation.MyCommand.Path} Import-Module $PSScriptRoot\CommonUtils.psm1 -Force +Import-Module OpenSSHUtils -Force $tC = 1 $tI = 0 @@ -8,26 +9,41 @@ $suite = "userenvironment" Describe "E2E scenarios for user environment block" -Tags "CI" { BeforeAll { if ($OpenSSHTestInfo -eq $null) { - Throw "`$OpenSSHTestInfo is null. Please run Set-OpenSSHTestEnvironment to set test environments." + throw "`$OpenSSHTestInfo is null. Please run Set-OpenSSHTestEnvironment to set test environments." } $server = $OpenSSHTestInfo["Target"] $port = $OpenSSHTestInfo["Port"] - $ssouser = $OpenSSHTestInfo["SSOUser"] $testDir = Join-Path $OpenSSHTestInfo["TestDataPath"] $suite if (-not (Test-Path $testDir)) { $null = New-Item $testDir -ItemType directory -Force -ErrorAction SilentlyContinue } - # Helper: run a one-liner inside an SSH session as the SSO user using PowerShell. - function Invoke-RemotePS { - param([Parameter(Mandatory=$true)][string] $Script) - $encoded = [Convert]::ToBase64String([System.Text.Encoding]::Unicode.GetBytes($Script)) - $out = ssh -p $port -o "StrictHostKeyChecking=no" -o "UserKnownHostsFile=$testDir\known_hosts" ` - "$ssouser@$server" powershell -NoProfile -NonInteractive -EncodedCommand $encoded 2>$null - if ($out -is [array]) { return ($out -join "`n").Trim() } - return ($out | Out-String).Trim() + $script:envTestUser = $OpenSSHTestInfo["SshdUser"] + $script:envTestProfile = $OpenSSHTestInfo["SshdUserProfile"] + + $keypassphrase = "testpassword" + $script:envTestKey = Join-Path $testDir "sshd_user_envtest_ed25519" + Remove-Item -Path "$($script:envTestKey)*" -Force -ErrorAction SilentlyContinue + ssh-keygen.exe -t ed25519 -f $script:envTestKey -P $keypassphrase + $envTestSshDir = Join-Path $script:envTestProfile .ssh + if (-not (Test-Path $envTestSshDir -PathType Container)) { + New-Item $envTestSshDir -ItemType Directory -Force -ErrorAction Stop | Out-Null + } + $script:envTestAuthKeys = Join-Path $envTestSshDir authorized_keys + Copy-Item "$($script:envTestKey).pub" $script:envTestAuthKeys -Force + Repair-AuthorizedKeyPermission -FilePath $script:envTestAuthKeys -confirm:$false + Add-PasswordSetting -Pass $keypassphrase + } + + AfterAll { + Remove-PasswordSetting + if ($script:envTestKey) { + Remove-Item -Path "$($script:envTestKey)*" -Force -ErrorAction SilentlyContinue + } + if ($script:envTestAuthKeys -and (Test-Path $script:envTestAuthKeys)) { + Remove-Item $script:envTestAuthKeys -Force -ErrorAction SilentlyContinue } } @@ -38,80 +54,77 @@ Describe "E2E scenarios for user environment block" -Tags "CI" { AfterAll { $tC++ } It "$tC.$tI - USERNAME matches the connecting user" { - $shortName = ($ssouser -split '\\')[-1] - $remote = Invoke-RemotePS '$env:USERNAME' - $remote | Should Be $shortName + $o = ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %USERNAME% + "$o".Trim() | Should Be $script:envTestUser } It "$tC.$tI - USERPROFILE points to the connecting user's profile" { - $remote = Invoke-RemotePS '$env:USERPROFILE' + $o = ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %USERPROFILE% + $remote = "$o".Trim() $remote | Should Not BeNullOrEmpty - # Reject the LocalSystem / sshd-session fallback profile. $remote | Should Not Match 'system32\\config\\systemprofile' - ($remote -split '\\')[-1] | Should Be (($ssouser -split '\\')[-1]) + # Profile leaf is normally the user name, but Windows can suffix + # it (e.g. sshd_user.DESKTOP-XYZ.001) when the profile dir was + # recreated, so just check it starts with the user name. + ($remote -split '\\')[-1] | Should Match ("^" + [regex]::Escape($script:envTestUser)) } - It "$tC.$tI - HOMEDRIVE and HOMEPATH are populated" { - $remote = Invoke-RemotePS '"{0}|{1}" -f $env:HOMEDRIVE, $env:HOMEPATH' - $parts = $remote -split '\|' - $parts[0] | Should Match '^[A-Za-z]:$' - $parts[1] | Should Not BeNullOrEmpty + It "$tC.$tI - HOMEDRIVE and HOMEPATH resolve to the user's profile" { + $hd = "$(ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %HOMEDRIVE%)".Trim() + $hp = "$(ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %HOMEPATH%)".Trim() + $up = "$(ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %USERPROFILE%)".Trim() + $hp | Should Not Match 'system32\\config\\systemprofile' + $hp | Should Not Match '^\\Windows' + $hp | Should Match ("^\\Users\\" + [regex]::Escape($script:envTestUser)) + $hd | Should Be $env:SystemDrive + ($hd + $hp) | Should Be $up } - It "$tC.$tI - APPDATA and LOCALAPPDATA are populated and per-user" { - $remote = Invoke-RemotePS '"{0}|{1}" -f $env:APPDATA, $env:LOCALAPPDATA' - $parts = $remote -split '\|' - $parts[0] | Should Match 'AppData\\Roaming$' - $parts[1] | Should Match 'AppData\\Local$' - $parts[0] | Should Not Match 'system32\\config\\systemprofile' - $parts[1] | Should Not Match 'system32\\config\\systemprofile' + It "$tC.$tI - APPDATA is populated for user" { + $ad = "$(ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %APPDATA%)".Trim() + $ad | Should Not Match 'system32\\config\\systemprofile' + $ad | Should Match 'AppData\\Roaming$' + $ad | Should Match ([regex]::Escape($script:envTestUser)) } - It "$tC.$tI - USERDOMAIN is populated and is not WORKGROUP" { - $remote = Invoke-RemotePS '$env:USERDOMAIN' - $remote | Should Not BeNullOrEmpty + It "$tC.$tI - LOCALAPPDATA is populated for user" { + $la = "$(ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %LOCALAPPDATA%)".Trim() + $la | Should Not Match 'system32\\config\\systemprofile' + $la | Should Match 'AppData\\Local$' + $la | Should Match ([regex]::Escape($script:envTestUser)) + } + + It "$tC.$tI - USERDOMAIN equals the local computer name" { + $o = ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %USERDOMAIN% + $remote = "$o".Trim() $remote | Should Not Be 'WORKGROUP' + $remote | Should Be $env:COMPUTERNAME } } - Context "$tC - PATH is the merged System + User PATH (Phase 1 fix)" { + Context "$tC - PATH variable" { BeforeAll { $tI = 1 - # Write a unique marker into the SSO user's HKCU\Environment\Path - # from within an SSH session (we are not the user locally, so we - # cannot edit their HKCU directly without loading the hive). $script:pathMarker = "C:\sshtestmarker_$([guid]::NewGuid().ToString('N'))" - $setScript = @" -`$existing = (Get-ItemProperty -Path 'HKCU:\Environment' -Name Path -ErrorAction SilentlyContinue).Path -if (-not `$existing) { `$existing = '' } -if (-not (`$existing -split ';' -contains '$($script:pathMarker)')) { - `$new = if (`$existing) { `$existing.TrimEnd(';') + ';$($script:pathMarker)' } else { '$($script:pathMarker)' } - Set-ItemProperty -Path 'HKCU:\Environment' -Name Path -Value `$new -Type ExpandString -} -"@ - Invoke-RemotePS $setScript | Out-Null + ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" ` + reg add HKCU\Environment /v Path /t REG_EXPAND_SZ /d $($script:pathMarker) /f | Out-Null } AfterAll { - $clearScript = @" -`$existing = (Get-ItemProperty -Path 'HKCU:\Environment' -Name Path -ErrorAction SilentlyContinue).Path -if (`$existing) { - `$new = ((`$existing -split ';') | Where-Object { `$_ -ne '$($script:pathMarker)' }) -join ';' - Set-ItemProperty -Path 'HKCU:\Environment' -Name Path -Value `$new -Type ExpandString -} -"@ - Invoke-RemotePS $clearScript | Out-Null + ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" ` + reg delete HKCU\Environment /v Path /f | Out-Null $tC++ } - It "$tC.$tI - PATH contains BOTH the system entry and the user marker" { - $remote = Invoke-RemotePS '$env:PATH' + It "$tC.$tI - contains both system and user entries" { + $o = ssh -p $port -i $script:envTestKey "$($script:envTestUser)@$server" echo %PATH% + $remote = "$o".Trim() $segments = $remote -split ';' - ($segments | Where-Object { $_ -match '[Ss]ystem32' }).Count | Should BeGreaterThan 0 - ($segments | Where-Object { $_ -eq $script:pathMarker }).Count | Should BeGreaterThan 0 - $sysIndex = ($segments | ForEach-Object { $_ }).IndexOf((($segments | Where-Object { $_ -match '[Ss]ystem32' }) | Select-Object -First 1)) - $markerIndex = ($segments | ForEach-Object { $_ }).IndexOf($script:pathMarker) - $sysIndex | Should BeGreaterThan -1 - $markerIndex | Should BeGreaterThan -1 + $sysMatches = @($segments | Where-Object { $_ -match '[Ss]ystem32' }) + $markerMatches = @($segments | Where-Object { $_ -eq $script:pathMarker }) + $sysMatches.Count | Should BeGreaterThan 0 + $markerMatches.Count | Should BeGreaterThan 0 + $sysIndex = [array]::IndexOf($segments, $sysMatches[0]) + $markerIndex = [array]::IndexOf($segments, $script:pathMarker) $sysIndex | Should BeLessThan $markerIndex } } From fdecfb154849b805ab7dfa03bcde8b3c6ff4ce9e Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 19 May 2026 14:48:46 -0400 Subject: [PATCH 3/4] revert unnecessary path merging --- contrib/win32/win32compat/w32fd.c | 89 ++----------------------------- 1 file changed, 5 insertions(+), 84 deletions(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 2420ef179a0a..dd6f1ff2faf6 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -1088,57 +1088,6 @@ is_sshd_service_token(HANDLE token) return is_sshd; } -static wchar_t* -override_path_in_env_block(const wchar_t* src, const wchar_t* merged_path) -{ - if (!src || !merged_path) - return NULL; - - const size_t prefix_len = 5; /* L"PATH=" */ - size_t merged_path_len = wcslen(merged_path); - size_t path_entry_chars = prefix_len + merged_path_len + 1; /* include trailing NUL */ - - /* Measure source block length (in wchars) including the final empty string. */ - const wchar_t* p = src; - while (*p) - p += wcslen(p) + 1; - size_t src_chars = (size_t)(p - src) + 1; /* +1 for the terminating empty string */ - - /* Worst case: original block plus one extra PATH entry. */ - size_t out_capacity = src_chars + path_entry_chars; - wchar_t* out = (wchar_t*)malloc(out_capacity * sizeof(wchar_t)); - if (!out) - return NULL; - - wchar_t* w = out; - BOOL replaced = FALSE; - const wchar_t* cur = src; - while (*cur) { - size_t entry_chars = wcslen(cur); /* excludes NUL */ - if (!replaced && entry_chars >= prefix_len && _wcsnicmp(cur, L"PATH=", prefix_len) == 0) { - memcpy(w, L"PATH=", prefix_len * sizeof(wchar_t)); - w += prefix_len; - memcpy(w, merged_path, merged_path_len * sizeof(wchar_t)); - w += merged_path_len; - *w++ = L'\0'; - replaced = TRUE; - } else { - memcpy(w, cur, (entry_chars + 1) * sizeof(wchar_t)); - w += entry_chars + 1; - } - cur += entry_chars + 1; - } - if (!replaced) { - memcpy(w, L"PATH=", prefix_len * sizeof(wchar_t)); - w += prefix_len; - memcpy(w, merged_path, merged_path_len * sizeof(wchar_t)); - w += merged_path_len; - *w++ = L'\0'; - } - *w = L'\0'; /* final double-NUL terminator */ - return out; -} - /* * spawn a child process * - specified by cmd with agruments argv @@ -1192,7 +1141,6 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, if (as_user) { debug3("spawning %ls as user", t); LPVOID lpEnvironment = NULL; - wchar_t* merged_env = NULL; if (!is_sshd_service_token(as_user)) { /* Load the user's environment block (HKCU vars, USERPROFILE, etc.), * inheriting the current context so session state set in @@ -1200,40 +1148,13 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, * virtual account (sshd worker chain re-spawning itself). */ CreateEnvironmentBlock(&lpEnvironment, as_user, TRUE); } - - if (lpEnvironment) { - /* Restore the merged System+User PATH that setup_session_user_vars - * computed into this process's PATH; CreateEnvironmentBlock - * overlays HKCU PATH on top of it, so we must override the block. */ - DWORD merged_path_chars = GetEnvironmentVariableW(L"PATH", NULL, 0); - if (merged_path_chars > 0) { - wchar_t* merged_path = (wchar_t*)malloc(merged_path_chars * sizeof(wchar_t)); - if (merged_path) { - if (GetEnvironmentVariableW(L"PATH", merged_path, merged_path_chars) > 0) - merged_env = override_path_in_env_block((const wchar_t*)lpEnvironment, merged_path); - free(merged_path); - } - } - } - - if (merged_env) { - b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, - flags | CREATE_UNICODE_ENVIRONMENT, - merged_env, NULL, &si, &pi); - free(merged_env); - } - else if (lpEnvironment) { - b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, - flags | CREATE_UNICODE_ENVIRONMENT, - lpEnvironment, NULL, &si, &pi); + if (lpEnvironment) { /* Pass the user environment block to the new process. */ + b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags | CREATE_UNICODE_ENVIRONMENT, lpEnvironment, NULL, &si, &pi); + DestroyEnvironmentBlock(lpEnvironment); } - else { - /* Fallback: pass the current context's environment block. */ - b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, - flags, NULL, NULL, &si, &pi); + else { /* Pass the current context's environment block to the new process. */ + b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags, NULL, NULL, &si, &pi); } - if (lpEnvironment) - DestroyEnvironmentBlock(lpEnvironment); } else { debug3("spawning %ls as subprocess", t); From 2a538e2687d2ef9ffe1f38ec7506d9bcdc022db5 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 19 May 2026 16:24:30 -0400 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- contrib/win32/win32compat/w32fd.c | 4 ++-- regress/pesterTests/UserEnvironment.Tests.ps1 | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index dd6f1ff2faf6..358f14c04971 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -1060,8 +1060,8 @@ is_sshd_service_token(HANDLE token) { BOOL is_sshd = FALSE; DWORD count = 0; - GetTokenInformation(token, TokenUser, NULL, 0, &count); - if (!count) + if (GetTokenInformation(token, TokenUser, NULL, 0, &count) || + GetLastError() != ERROR_INSUFFICIENT_BUFFER || !count) return FALSE; void* buffer = malloc(count); if (!buffer) diff --git a/regress/pesterTests/UserEnvironment.Tests.ps1 b/regress/pesterTests/UserEnvironment.Tests.ps1 index 3708ed432b7d..1e301da0db21 100644 --- a/regress/pesterTests/UserEnvironment.Tests.ps1 +++ b/regress/pesterTests/UserEnvironment.Tests.ps1 @@ -26,7 +26,7 @@ Describe "E2E scenarios for user environment block" -Tags "CI" { $keypassphrase = "testpassword" $script:envTestKey = Join-Path $testDir "sshd_user_envtest_ed25519" Remove-Item -Path "$($script:envTestKey)*" -Force -ErrorAction SilentlyContinue - ssh-keygen.exe -t ed25519 -f $script:envTestKey -P $keypassphrase + ssh-keygen.exe -q -t ed25519 -f $script:envTestKey -N $keypassphrase $envTestSshDir = Join-Path $script:envTestProfile .ssh if (-not (Test-Path $envTestSshDir -PathType Container)) { New-Item $envTestSshDir -ItemType Directory -Force -ErrorAction Stop | Out-Null