Files
PowerToys/.pipelines/verifyDepsJsonLibraryVersions.ps1
Dave Rayment 3079a3c546 [EnvironmentVariables] Validation fixes, centralised validation, error message improvements (#46837)
## Summary of the Pull Request
This fixes several critical validation issues with the Environment
Variables utility, centralises the validation, guards registry writes,
and improves error messages for validation failures.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist

- [x] Closes: #46763
<!-- - [ ] Closes: #yyy (add separate lines for additional resolved
issues) -->
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [x] **Tests:** Added/updated and all pass
- [ ] **Localization:** All end-user-facing strings can be localized
- [ ] **Dev docs:** Added/updated
- [ ] **New binaries:** Added on the required places
- [ ] [JSON for
signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json)
for new binaries
- [ ] [WXS for
installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs)
for new binaries and localization folder
- [ ] [YML for CI
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml)
for new test projects
- [ ] [YML for signed
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml)
- [ ] **Documentation updated:** If checked, please file a pull request
on [our docs
repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys)
and link it here: #xxx

<!-- Provide a more detailed description of the PR, other things fixed,
or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
This PR fixes a reported critical vulnerability (#46763) where creating
an environment variable with an equals sign in the name reportedly
caused Windows to crash and enter a boot loop upon restarting. During
the investigation of the issue, several other environment variable
constraints were identified as missing, including there being no
prevention of leading or trailing spaces in names (meaning variables
could not be typed on the command line), no combined length checks and
so on. This PR introduces a centralised, robust validation pipeline for
UI and registry writes to prevent entering states which could corrupt
the Windows environment block.

## Changes
### Centralised validation logic
- All environment validation is now inside
EnvironmentVariablesHelper.cs, rather than split between this code and
the UI.
- Both the UI (via model validate bindings) and backend registry writes
now strictly evaluate against the same unified ruleset before applying
changes or enabling/disabling controls.
- Existing methods have been updated to report back their success or
failure, to enable errors to be tracked more effectively.

### Blocked OS-breaking characters
- In response to the user report, the equals character is now blocked
from both Variable and Profile names. `=` being disallowed is
[explicitly
mentioned](https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables)
in the Environment Variables Win32 documentation, so it's a surprise it
wasn't caught previously.
- All control characters (including `\0`, `\r` and `\n`) are also
disallowed, both to protect the integrity of the environment block and
the rendering of the strings in the UI.
- Leading and trailing whitespace is rejected to prevent orphaned
variables.

### Enforced Windows length constraints
- Variable Names and Profile Names are restricted to 259 characters, to
match the 260-character null-terminated string length limit in the
Windows Environment Variables Editor (via sysdm.cpl) and RegEdit. To be
clear: profile names may technically be longer, but we should choose to
abide by this authoring tool limit to maintain compatibility with other
editors. There was previously a 255-character limit on names in the
code, and a comment indicating this was a registry limit, but that was
incorrect and has been removed. The new limit constant is
`MaxEnvironmentVariableNameAuthoringLength`.
- In the prior code, there was no limit on the length of system variable
names. This was incorrect. The limit for both System and User name
fields is now the identical at 259 characters.
- There is a length limit on the full environment variable entry
`[VariableName]=[VariableValue]\0`, which is 32766 characters plus the
null-terminator. This is now enforced and the constant is
`MaxTotalEnvironmentVariableLength`. (There's no imposed limit on the
number of environment variables.)

### Fixed User Profile backup "overflows"
- Fixed a bug where a user could create a valid Profile Name and a valid
environment variable name, but applying them would silently fail to
apply the profile because the generated backup variable name
`[VariableName]_PowerToys_[ProfileName]` exceeded the previous authoring
limit of 255 characters.
- Backup variables are excluded from the 259-character limit, as they
are internal to the application, but the combined
`[VariableName]=[VariableVavlue]\0` length is still strictly constrained
to the 32767 environment variable length limit.

There are now separate paths through the code to deal with backup
variable persistence and validation.

### UI
- If an applied user profile's name is now rejected because of the new
rules (e.g. it contains `=`), the UI now shows a specific "Profile name
is invalid" warning rather than the generic "not applicable" message
from before, allowing the user to identify and fix the problem.
- Fixed a small issue in the Add New Variable dialog where a vertical
scrollbar was always present. There are other cases where this occurs,
too, but I've left them for a future PR.

<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed

New unit tests project added with coverage of the new validation
functionality.

Also, manual testing...

Manual validation of each dialog:
- Add variable
- Edit variable
- Add variable via Profile Edit dialog

Test against:
- `=` being in either the Variable Name or the Profile Name
- A control character being present in the Variable Name or Profile Name
- Either the Variable Name or Profile Name containing one or more
trailing or leading whitespace characters
- The length of the Variable Name being longer than 259 characters
- The combined length of name + `=` + value being longer than 32766
characters

The dialog tests can be confirmed by checking to see if the Save button
is enabled:

<img width="1099" height="843" alt="image"
src="https://github.com/user-attachments/assets/685e561a-bd8d-4926-b9b2-a61dea4cc96a"
/>

Also confirm:
- An invalid Profile Name is caught. This can be confirmed by:

Editing the JSON file and adding an `=` character in the name:
<img width="497" height="197" alt="image"
src="https://github.com/user-attachments/assets/c6aa5d62-0672-499a-aac4-c639e8158b61"
/>

Then opening the application and trying to enable the profile:

<img width="1117" height="371" alt="image"
src="https://github.com/user-attachments/assets/bd887a44-5e65-4750-9c6f-9bf1b82a5ad6"
/>

Also confirm that in the Edit profile dialog, you can enable the
profile, but the Save button is disabled:

<img width="688" height="603" alt="image"
src="https://github.com/user-attachments/assets/10d186d9-17a0-4210-93e3-23b1e2723f5f"
/>

- Confirm that control characters cannot be part of the Variable Name:

First, run this from PowerShell, which adds a string containing the
newline character to the clipboard:

```pwsh
Set-Clipboard -Value "MyVar`nName"
```

Open the Add or Edit variable dialog and paste the value into the Name
field. Confirm that the character is not pasted and the string truncates
before it:

<img width="1424" height="732" alt="image"
src="https://github.com/user-attachments/assets/260ff728-57a2-438f-bb66-08d32a327b64"
/>

(For the null character specifically, use `Set-Clipboard -Value ("MyVar"
+ [char]0 + "Name")`.)

- The initial dialog button state. Re-open the Add New variable dialog
multiple times and confirm the Save button is disabled each time before
making any input.

- In the Add/Edit Variable dialogs, enter a valid variable name and then
clear it, confirming that the Save button enables and disables
correctly.

## Still outstanding

There are some flaws I've found which I'm choosing to leave for now,
mainly for expedience so the above issues can be prioritised:
- Handling duplicate profile names - there is the potential there for
duplicate variable names under identically-named profiles to conflict.
- Profile JSON import is still not sanitised.

These should be added in a future PR.
2026-08-05 16:42:25 +08:00

98 lines
5.1 KiB
PowerShell

[CmdletBinding()]
Param(
[Parameter(Mandatory = $True, Position = 1)]
[string]$targetDir
)
# This script will check every deps.json file in the target directory to see if for each dll mentioned,
#all the deps.json files that mention it will mention the same version.
# The main goal is to catch when different versions for the same module might be copied to the same directory
#at build time and might create flaky builds that get the wrong version of the dll sometimes.
# A dictionary of dictionaries of lists to save which files reference each version of each dll.
# Logic is DllName > fileVersion > list with deps.json files that reference it.
# If for a specific dll there's more than one referenced file version, we have build collisions.
$referencedFileVersionsPerDll = @{}
$totalFailures = 0
Get-ChildItem $targetDir -Recurse -Filter *.deps.json -Exclude *UITest*,MouseJump.Common.UnitTests*,EnvironmentVariablesUILib.UnitTests*,*.FuzzTests* | ForEach-Object {
# Temporarily exclude All UI-Test, Fuzzer-Test projects because of Appium.WebDriver dependencies.
# MouseJump.Common.UnitTests and EnvironmentVariablesUILib.UnitTests are self-contained WinUI (CsWinRT) unit tests:
# each bundles its full runtime closure into an isolated tests\<name> output folder, so its private dll copies
# cannot collide with product binaries at runtime and are safe to skip in this cross-dependency version check.
$depsJsonFullFileName = $_.FullName
if ($depsJsonFullFileName -like "*CmdPal*" -or $depsJsonFullFileName -like "*CommandPalette*") {
return
}
$depsJsonFileName = $_.Name
$depsJson = Get-Content $depsJsonFullFileName | ConvertFrom-Json
# We're doing a breadth first search to look for every runtime object.
$iterateThroughEveryField = New-Object System.Collections.Generic.Queue[System.Object]
$iterateThroughEveryField.Enqueue($depsJson)
while($iterateThroughEveryField.Count -gt 0)
{
$currentObject = $iterateThroughEveryField.Dequeue();
$currentObject.PSObject.Properties | ForEach-Object {
if($_.Name -ne 'SyncRoot') {
# Skip SyncRoot to avoid looping in array objects.
# Care only about objects, not value types.
$iterateThroughEveryField.Enqueue($_.Value)
if($_.Name -eq 'runtime')
{
# Cycle through each dll.
$_.Value.PSObject.Properties | ForEach-Object {
if($_.Name.EndsWith('.dll')) {
$dllName = Split-Path $_.Name -leaf
if([bool]($_.Value.PSObject.Properties.name -match 'fileVersion')) {
$dllFileVersion = $_.Value.fileVersion
if (([string]::IsNullOrEmpty($dllFileVersion) -or ($dllFileVersion -eq '0.0.0.0')) -and $dllName.StartsWith('PowerToys.'))` {
# After VS 17.11 update some of PowerToys dlls have no fileVersion in deps.json even though the
# version is correctly set. This is a workaround to skip our dlls as we are confident that all of
# our dlls share the same version across the dependencies.
# After VS 17.13 these error versions started appearing as 0.0.0.0 so we've added that case to the condition as well.
continue
}
# Add the entry to the dictionary of dictionary of lists
if(-Not $referencedFileVersionsPerDll.ContainsKey($dllName)) {
$referencedFileVersionsPerDll[$dllName] = @{ $dllFileVersion = New-Object System.Collections.Generic.List[System.String] }
} elseif(-Not $referencedFileVersionsPerDll[$dllName].ContainsKey($dllFileVersion)) {
$referencedFileVersionsPerDll[$dllName][$dllFileVersion] = New-Object System.Collections.Generic.List[System.String]
}
$referencedFileVersionsPerDll[$dllName][$dllFileVersion].Add($depsJsonFileName)
}
}
}
}
}
}
}
}
# Report on the files that are referenced for more than one version.
$referencedFileVersionsPerDll.keys | ForEach-Object {
if($referencedFileVersionsPerDll[$_].Count -gt 1) {
$dllName = $_
Write-Host $dllName
$referencedFileVersionsPerDll[$dllName].keys | ForEach-Object {
Write-Host "`t" $_
$referencedFileVersionsPerDll[$dllName][$_] | ForEach-Object {
Write-Host "`t`t" $_
}
}
$totalFailures++;
}
}
if ($totalFailures -gt 0) {
Write-Host -ForegroundColor Red "Detected " $totalFailures " libraries that are mentioned with different version across the dependencies.`r`n"
exit 1
}
Write-Host -ForegroundColor Green "All " $referencedFileVersionsPerDll.keys.Count " libraries are mentioned with the same version across the dependencies.`r`n"
exit 0