Skip to content

Commit bb29926

Browse files
localconstlocalconst
authored
Update PowerShellV2 sanitize regexp (#19053)
* Add \n to allowed symbols * Bump task to 2.229.2 * Separate test suites * Fix naming * Update regex * Update regex for node version * Exclude A-Z from regex * Use \w instead of a-z0-9 * Revert msbuildV1 changes * bump task version
1 parent e8bab02 commit bb29926

26 files changed

+272
-176
lines changed

‎BuildConfigGen/Program.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ private static void UpdateVersions(string gitRootPath, string task, string taskT
570570

571571
if (inputVersion <= maxVersion && !defaultVersionMatchesSourceVersion)
572572
{
573-
throw new Exception($"inputVersion={inputVersion} version specified in task taskTarget={taskTarget} must not be less or equal to maxversion maxVersion={maxVersion} specified in versionMapFile{versionMapFile}, or must match defaultVersion={defaultVersion} in {versionMapFile}");
573+
throw new Exception($"inputVersion={inputVersion} version specified in task taskTarget={taskTarget} must not be less or equal to maxversion maxVersion={maxVersion} specified in versionMapFile {versionMapFile}, or must match defaultVersion={defaultVersion} in {versionMapFile}");
574574
}
575575
}
576576

‎Tasks/PowerShellV2/Tests/L0ValidateFileArgs.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,17 @@ export const runValidateFileArgsTests = () => {
2222
],
2323
[
2424
"Accepts allowed symbols",
25-
"a A 1 \\ ` _ ' \" - = / : . * , + ~ ? %", ["AZP_75787_ENABLE_NEW_LOGIC=true"]
25+
"a A z Z 1 \\ ` _ ' \" - = / : . * , + ~ ? % \n #", ["AZP_75787_ENABLE_NEW_LOGIC=true"]
26+
],
27+
[
28+
"Paths check",
29+
"D:\\my\\path d/my/path",
30+
["AZP_75787_ENABLE_NEW_LOGIC=true"]
31+
],
32+
[
33+
"Accepts $true and $false",
34+
"$TrUe $true $fAlsE $false",
35+
["AZP_75787_ENABLE_NEW_LOGIC=true"]
2636
]
2737
];
2838

‎Tasks/PowerShellV2/Tests/powershellImpl/L0.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,12 @@ export function testPowerShellImpl() {
2525
psr.run(path.join(__dirname, 'L0Expand-EnvVariables.ps1'), done);
2626
})
2727

28-
it('Run of L0Test-FileArgs tests suite.', (done) => {
29-
psr.run(path.join(__dirname, 'L0Test-FileArgs.ps1'), done);
28+
it('Run of L0Test-FileArgs.Passes tests suite.', (done) => {
29+
psr.run(path.join(__dirname, 'L0Test-FileArgs.Passes.ps1'), done);
30+
})
31+
32+
it('Run of L0Test-FileArgs.Fails tests suite.', (done) => {
33+
psr.run(path.join(__dirname, 'L0Test-FileArgs.Fails.ps1'), done);
3034
})
3135
}
3236
}

‎Tasks/PowerShellV2/Tests/powershellImpl/L0Test-FileArgs.Fails.ps1‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
[CmdletBinding()]
2+
param()
3+
4+
. $PSScriptRoot\..\..\..\..\Tests\lib\Initialize-Test.ps1
5+
. $PSScriptRoot\..\..\helpers.ps1
6+
7+
$failTestSuites = @(
8+
@{
9+
Name = 'If dangerous symbols are present, and FF is on'
10+
Input = 'test; whoami'
11+
Variables = @('AZP_75787_ENABLE_NEW_LOGIC=true')
12+
},
13+
@{
14+
Name = 'If inside args line is env variable with dangerous symbols'
15+
Input = 'test $env:VAR1 test'
16+
Variables = @('VAR1=12;3', 'AZP_75787_ENABLE_NEW_LOGIC=true')
17+
},
18+
@{
19+
Name = 'If inside args line not correct env syntax'
20+
Input = 'test $venv:VAR1 test'
21+
Variables = @('VAR1=123', 'AZP_75787_ENABLE_NEW_LOGIC=true')
22+
}
23+
)
24+
foreach ($test in $failTestSuites) {
25+
$test.Variables | ForEach-Object {
26+
$name, $value = $_.Split('=')
27+
if ($value) {
28+
Set-Item -Path env:$name -Value $value
29+
}
30+
else {
31+
Remove-Item env:$name -ErrorAction SilentlyContinue
32+
}
33+
}
34+
35+
try {
36+
$msg = Get-VstsLocString -Key 'ScriptArgsSanitized'
37+
Assert-Throws {
38+
Test-FileArgs $test.Input
39+
} -MessagePattern $msg
40+
}
41+
catch {
42+
throw "Error occured in '$($test.Name)' suite: $($_.Exception.Message)"
43+
}
44+
finally {
45+
$test.Variables | ForEach-Object {
46+
$name, $value = $_.Split('=')
47+
Remove-Item env:$name -ErrorAction SilentlyContinue
48+
}
49+
}
50+
}

‎Tasks/PowerShellV2/Tests/powershellImpl/L0Test-FileArgs.ps1‎ renamed to ‎Tasks/PowerShellV2/Tests/powershellImpl/L0Test-FileArgs.Passes.ps1‎

Lines changed: 11 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ param()
44
. $PSScriptRoot\..\..\..\..\Tests\lib\Initialize-Test.ps1
55
. $PSScriptRoot\..\..\helpers.ps1
66

7-
$notThrowTestSuites = @(
7+
$passTestSuites = @(
88
@{
99
Name = 'Handles empty line'
1010
Input = ''
@@ -27,53 +27,21 @@ $notThrowTestSuites = @(
2727
},
2828
@{
2929
Name = 'Accepts allowed symbols'
30-
Input = 'a A 1 \ ` _ '' " - = / : . * , + ~ ? %'
31-
Variables = @('AZP_75787_ENABLE_NEW_LOGIC=true')
32-
}
33-
)
34-
foreach ($test in $notThrowTestSuites) {
35-
$test.Variables | ForEach-Object {
36-
$name, $value = $_.Split('=')
37-
if ($value) {
38-
Set-Item -Path env:$name -Value $value
39-
}
40-
else {
41-
Remove-Item env:$name -ErrorAction SilentlyContinue
42-
}
43-
}
44-
45-
try {
46-
Test-FileArgs $test.Input
47-
}
48-
catch {
49-
throw "Error occured in '$($test.Name)' suite: $($_.Exception.Message)"
50-
}
51-
finally {
52-
$test.Variables | ForEach-Object {
53-
$name, $value = $_.Split('=')
54-
Remove-Item env:$name -ErrorAction SilentlyContinue
55-
}
56-
}
57-
}
58-
59-
$throwTestSuites = @(
60-
@{
61-
Name = 'If dangerous symbols are present, and FF is on'
62-
Input = 'test; whoami'
30+
Input = "a A z Z 1 \ ` _ ' `" - = / : . * , + ~ ? % `n #"
6331
Variables = @('AZP_75787_ENABLE_NEW_LOGIC=true')
6432
},
6533
@{
66-
Name = 'If inside args line is env variable with dangerous symbols'
67-
Input = 'test $env:VAR1 test'
68-
Variables = @('VAR1=12;3', 'AZP_75787_ENABLE_NEW_LOGIC=true')
34+
Name = 'Paths check'
35+
Input = 'D:\my\path d/my/path'
36+
Variables = @('AZP_75787_ENABLE_NEW_LOGIC=true')
6937
},
7038
@{
71-
Name = 'If inside args line not correct env syntax'
72-
Input = 'test $venv:VAR1 test'
73-
Variables = @('VAR1=123', 'AZP_75787_ENABLE_NEW_LOGIC=true')
39+
Name = 'Accepts $true and $false'
40+
Input = '$TrUe $true $fAlsE $false'
41+
Variables = @('AZP_75787_ENABLE_NEW_LOGIC=true')
7442
}
7543
)
76-
foreach ($test in $throwTestSuites) {
44+
foreach ($test in $passTestSuites) {
7745
$test.Variables | ForEach-Object {
7846
$name, $value = $_.Split('=')
7947
if ($value) {
@@ -85,10 +53,7 @@ foreach ($test in $throwTestSuites) {
8553
}
8654

8755
try {
88-
$msg = Get-VstsLocString -Key 'ScriptArgsSanitized'
89-
Assert-Throws {
90-
Test-FileArgs $test.Input
91-
} -MessagePattern $msg
56+
Test-FileArgs $test.Input
9257
}
9358
catch {
9459
throw "Error occured in '$($test.Name)' suite: $($_.Exception.Message)"
@@ -100,3 +65,4 @@ foreach ($test in $throwTestSuites) {
10065
}
10166
}
10267
}
68+

‎Tasks/PowerShellV2/helpers.ps1‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ function Sanitize-Arguments([string]$InputArgs) {
3232
# We're splitting by ``, removing all suspicious characters and then join
3333
$argsArr = $InputArgs -split $argsSplitSymbols;
3434

35-
## '?<!`' - checking if before character no backtick. '^a-zA-Z0-9` _'"-=\/:\.*,+~?' - checking if character is allowed. Insead replacing to #removed#
36-
$regex = '(?<!`)([^a-zA-Z0-9\\` _''"\-=\/:\.*,+~?%])'
35+
## PowerShell Regex is case insensitive by default, so we don't need to specify a-zA-Z.
36+
## ('?<!`') - checking if before character no backtick.
37+
## ([^\w` _'"-=\/:\.*,+~?%\n#]) - checking if character is allowed. Insead replacing to #removed#
38+
$regex = '(?<!`)([^\w\\` _''"\-=\/:\.*,+~?%\n#])(?!true|false)'
3739
for ($i = 0; $i -lt $argsArr.Length; $i++ ) {
3840
[string[]]$matches = (Select-String $regex -input $argsArr[$i] -AllMatches) | ForEach-Object { $_.Matches }
3941
if ($null -ne $matches ) {

‎Tasks/PowerShellV2/helpers.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ export function validateFileArgs(inputArguments: string): void {
180180
expandedArgs,
181181
{
182182
argsSplitSymbols: '``',
183-
saniziteRegExp: new RegExp('(?<!`)([^a-zA-Z0-9\\\\` _\'"\\-=\\/:\\.*,+~?%])', 'g')
183+
saniziteRegExp: new RegExp('(?<!`)([^\\w\\\\` _\'"\\-=\\/:\\.*,+~?%\\n#])(?!true|false)', 'ig')
184184
}
185185
);
186186
if (sanitizedArgs !== inputArguments) {

‎Tasks/PowerShellV2/task.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
"author": "Microsoft Corporation",
1818
"version": {
1919
"Major": 2,
20-
"Minor": 229,
21-
"Patch": 4
20+
"Minor": 230,
21+
"Patch": 0
2222
},
2323
"releaseNotes": "Script task consistency. Added support for macOS and Linux.",
2424
"minimumAgentVersion": "2.115.0",

‎Tasks/PowerShellV2/task.loc.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
"author": "Microsoft Corporation",
1818
"version": {
1919
"Major": 2,
20-
"Minor": 229,
21-
"Patch": 4
20+
"Minor": 230,
21+
"Patch": 0
2222
},
2323
"releaseNotes": "ms-resource:loc.releaseNotes",
2424
"minimumAgentVersion": "2.115.0",

‎_generated/PowerShellV2.versionmap.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
Default|2.229.4
2-
Node20-225|2.229.5
1+
Default|2.230.0
2+
Node20-225|2.230.1

0 commit comments

Comments
 (0)