diff --git a/CHANGELOG.md b/CHANGELOG.md index ba5d279..110171a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,19 @@ release notes. ## [Unreleased] +### Added + +- `Repair-IntuneScript` applies eight more edits, each the one the finding's message asks for: `-Force` on + `Install-Module`, `Install-PackageProvider`, `Install-Package`, `Update-Module` and `Uninstall-Module`, + `-Confirm:$false` on `Register-PSRepository`, `-ErrorAction SilentlyContinue` on a probing cmdlet in a Win32 + detection or requirement script, `exit 1` for an exit code other than 0 or 1, `$env:ProgramW6432` for + `$env:ProgramFiles`, `'ARM64|AMD64'` for `'AMD64'` as a `-match` pattern, `$PSScriptRoot` for `$PWD`, a + `Get-Credential -Credential` call handed a credential that can only be one already built replaced by that + credential, and a `Set-ExecutionPolicy` statement or a `#Requires -Version 7` line removed. An edit that + would leave a broken statement is withheld and the finding stays: `Set-ExecutionPolicy` in a pipeline, + `'AMD64'` compared with `-eq`, `$PWD.Path`. The `#Requires` finding now sits on its line rather than on + the whole script. + ### Fixed - **`Test-IntuneDeployedScript` decided whether a group holds devices from its first 20 members.** A mixed group diff --git a/Private/Rules/Find-IslArchitectureIssue.ps1 b/Private/Rules/Find-IslArchitectureIssue.ps1 index 28735e3..cc0e264 100644 --- a/Private/Rules/Find-IslArchitectureIssue.ps1 +++ b/Private/Rules/Find-IslArchitectureIssue.ps1 @@ -89,6 +89,8 @@ function Find-IslArchitectureIssue { Message = ("`$env:ProgramFiles is 'Program Files (x86)' in the 32-bit host$suffix. Use " + "`$env:ProgramW6432 for the 64-bit folder") Evidence = $evidence + # ProgramW6432 is set in 32-bit and 64-bit processes alike on a 64-bit Windows + Fix = @{ Replacement = ($variable.Extent.Text -replace '(?i)ProgramFiles', 'ProgramW6432') } } New-IslFinding @findingSplat } diff --git a/Private/Rules/Find-IslArm64Assumption.ps1 b/Private/Rules/Find-IslArm64Assumption.ps1 index 13b3ae6..440b54d 100644 --- a/Private/Rules/Find-IslArm64Assumption.ps1 +++ b/Private/Rules/Find-IslArm64Assumption.ps1 @@ -46,6 +46,14 @@ function Find-IslArm64Assumption { "match 'ARM64|AMD64'") Evidence = $evidence } + # Only where the literal is the pattern of a -match: as the right side of -eq the alternation + # would match nothing at all + $parent = $literal.Parent + $matched = $parent.GetType().Name -eq 'BinaryExpressionAst' -and + "$($parent.Operator)" -in 'Imatch', 'Inotmatch', 'Cmatch', 'Cnotmatch' -and $parent.Right -eq $literal + if ($matched) { + $findingSplat.Fix = @{ Replacement = $literal.Extent.Text.Replace($literal.Value, 'ARM64|AMD64') } + } New-IslFinding @findingSplat } diff --git a/Private/Rules/Find-IslExecutionPolicyCall.ps1 b/Private/Rules/Find-IslExecutionPolicyCall.ps1 index dec9761..30e81bf 100644 --- a/Private/Rules/Find-IslExecutionPolicyCall.ps1 +++ b/Private/Rules/Find-IslExecutionPolicyCall.ps1 @@ -64,6 +64,12 @@ function Find-IslExecutionPolicyCall { Evidence = $evidence } } + # The call is removed when it is a statement on its own: in a pipeline, or as a value, taking + # it out would leave a broken statement with nothing to say so + $pipeline = $command.Parent + $alone = $pipeline.GetType().Name -eq 'PipelineAst' -and @($pipeline.PipelineElements).Count -eq 1 -and + $pipeline.Parent.GetType().Name -in 'NamedBlockAst', 'StatementBlockAst' + if ($alone) { $findingSplat.Fix = @{ Replacement = '' } } New-IslFinding @findingSplat } } diff --git a/Private/Rules/Find-IslExitCodeIssue.ps1 b/Private/Rules/Find-IslExitCodeIssue.ps1 index 92ecfa5..92d310c 100644 --- a/Private/Rules/Find-IslExitCodeIssue.ps1 +++ b/Private/Rules/Find-IslExitCodeIssue.ps1 @@ -107,6 +107,8 @@ function Find-IslExitCodeIssue { "again. Use exit 1 for 'issue found'") Evidence = ('exit 2 and exit -1 both triggered the remediation and ended as Recurred ' + '(REM-EXIT-2, REM-EXIT-NEG1); docs say only exit 1 does') + # Intune reads every non-zero exit as 1 already; writing it makes the report match + Fix = @{ Replacement = 'exit 1' } } New-IslFinding @findingSplat } diff --git a/Private/Rules/Find-IslInteractiveCall.ps1 b/Private/Rules/Find-IslInteractiveCall.ps1 index 6303036..6e84b55 100644 --- a/Private/Rules/Find-IslInteractiveCall.ps1 +++ b/Private/Rules/Find-IslInteractiveCall.ps1 @@ -168,6 +168,8 @@ function Find-IslInteractiveCall { "$source, so it is a credential that is already built and nothing prompts; the call " + 'can go') Evidence = $builtEvidence + # The call returns what it was handed, so the argument stands in for it + Fix = @{ Replacement = $handed.Extent.Text } } New-IslFinding @findingSplat continue @@ -241,6 +243,13 @@ function Find-IslInteractiveCall { "-Force / -Confirm:`$false or it hangs until the $timeout timeout") Evidence = $evidence } + # The switch the message asks for, added to the call. Set-ExecutionPolicy gets none: the + # agent launches with -ExecutionPolicy Bypass, and IslExecutionPolicyCall removes the call + if ($commandName -ne 'Set-ExecutionPolicy') { + $switch = if ($confirming[$commandName]) { " -$($confirming[$commandName])" } + else { ' -Confirm:$false' } + $findingSplat.Fix = @{ Replacement = $command.Extent.Text + $switch } + } New-IslFinding @findingSplat } } diff --git a/Private/Rules/Find-IslOutputIssue.ps1 b/Private/Rules/Find-IslOutputIssue.ps1 index 5be0a40..e17acaf 100644 --- a/Private/Rules/Find-IslOutputIssue.ps1 +++ b/Private/Rules/Find-IslOutputIssue.ps1 @@ -170,6 +170,7 @@ 'SilentlyContinue (or Test-Path first) on the not-installed path') Evidence = ('A non-terminating cmdlet error appeared in the captured error output ' + '(REM-EXIT-ERRNOEXIT); any stderr fails Win32 detection (W32-DET-STDERR)') + Fix = @{ Replacement = $unguarded[0].Extent.Text + ' -ErrorAction SilentlyContinue' } } New-IslFinding @findingSplat } @@ -283,6 +284,7 @@ 'target is missing, which fails the rule. Use -ErrorAction SilentlyContinue or ' + 'Test-Path first') Evidence = 'Any stderr fails the rule (W32-REQ-STDERR)' + Fix = @{ Replacement = $unguarded[0].Extent.Text + ' -ErrorAction SilentlyContinue' } } New-IslFinding @findingSplat } diff --git a/Private/Rules/Find-IslPowerShell7Syntax.ps1 b/Private/Rules/Find-IslPowerShell7Syntax.ps1 index 9c593d6..e8767a6 100644 --- a/Private/Rules/Find-IslPowerShell7Syntax.ps1 +++ b/Private/Rules/Find-IslPowerShell7Syntax.ps1 @@ -57,15 +57,21 @@ function Find-IslPowerShell7Syntax { $requires = $ast.ScriptRequirements if ($requires -and $requires.RequiredPSVersion -and $requires.RequiredPSVersion.Major -ge 6) { + # The requirement is a comment token, not a node; the finding sits on that line, and the + # fix takes the line out. What the script then does under 5.1 the other findings say + $requiresToken = @($Context.Tokens | Where-Object { + $_.Kind -eq 'Comment' -and $_.Text -match '(?i)^#requires\s+-version\b' + }) | Select-Object -First 1 $findingSplat = @{ RuleName = $rule Severity = 'Error' Context = $Context - Extent = $ast.Extent + Extent = if ($requiresToken) { $requiresToken.Extent } else { $ast.Extent } Message = ("'#Requires -Version $($requires.RequiredPSVersion)' cannot be satisfied: Intune runs " + 'Windows PowerShell 5.1, so the script exits 1 before its first line') Evidence = $requiresEvidence } + if ($requiresToken) { $findingSplat.Fix = @{ Replacement = '' } } New-IslFinding @findingSplat } diff --git a/Private/Rules/Find-IslRelativePath.ps1 b/Private/Rules/Find-IslRelativePath.ps1 index 4191ea0..5bad48e 100644 --- a/Private/Rules/Find-IslRelativePath.ps1 +++ b/Private/Rules/Find-IslRelativePath.ps1 @@ -56,6 +56,10 @@ function Find-IslRelativePath { 'the script') Evidence = $evidence } + # $PSScriptRoot is a string where $PWD is a PathInfo, so $PWD.Path and the like get no edit + if ($variable.Parent.GetType().Name -ne 'MemberExpressionAst') { + $findingSplat.Fix = @{ Replacement = ($variable.Extent.Text -replace '(?i)PWD', 'PSScriptRoot') } + } New-IslFinding @findingSplat } } diff --git a/README.md b/README.md index c3bdf6a..75924fb 100644 --- a/README.md +++ b/README.md @@ -169,7 +169,13 @@ Applied Remaining Written Fixes Path Three findings come with an edit the tool can make for you, each behaviour-preserving: a script-scope `return` becomes the `exit 0` it already produced, with any returned value written first (`return 'ok'` to `'ok'; exit 0`); a UTF-16 or BOM-less non-ASCII file is rewritten as UTF-8 -with a BOM; a padded requirement value (`' ok '`) is trimmed. `Repair-IntuneScript` applies them +with a BOM; a padded requirement value (`' ok '`) is trimmed. Another eight edits follow the +finding's own advice: `-Force` on `Install-Module` and its kin, `-Confirm:$false` on +`Register-PSRepository`, `-ErrorAction SilentlyContinue` on a probing cmdlet, `exit 1` for an exit +code Intune reads as 1 anyway, `$env:ProgramW6432` for `$env:ProgramFiles`, `'ARM64|AMD64'` as a +`-match` pattern, `$PSScriptRoot` for `$PWD`, a `Get-Credential -Credential` call that returns +what it was handed replaced by the argument, and a `Set-ExecutionPolicy` or `#Requires -Version 7` +line removed. `Repair-IntuneScript` applies them per script, reports what it changed and how many findings remain, previews with `-WhatIf`, and takes `-ScriptType`, `-Context`, `-Architecture` and `-EnforceSignatureCheck` the way `Test-IntuneScript` does. Whether that `exit 0` should have been an `exit 1` is still the author's call, which is why the diff --git a/Tests/Unit/Public/Repair-IntuneScript.Tests.ps1 b/Tests/Unit/Public/Repair-IntuneScript.Tests.ps1 index 6329953..414ef99 100644 --- a/Tests/Unit/Public/Repair-IntuneScript.Tests.ps1 +++ b/Tests/Unit/Public/Repair-IntuneScript.Tests.ps1 @@ -98,6 +98,73 @@ Describe 'Repair-IntuneScript' -Tag 'Unit', 'Public' { } } + Context 'Mechanical fixes' { + It 'applies the fix: becomes ' -ForEach @( + @{ Rule = 'IslInteractiveCall'; Type = 'Remediation'; Name = 'Remediate.ps1' + Before = 'Install-Module Foo'; After = 'Install-Module Foo -Force' } + @{ Rule = 'IslInteractiveCall'; Type = 'Remediation'; Name = 'Remediate.ps1' + Before = "Register-PSRepository -Name r -SourceLocation 'https://x'" + After = "Register-PSRepository -Name r -SourceLocation 'https://x' -Confirm:`$false" } + @{ Rule = 'IslInteractiveCall'; Type = 'Remediation'; Name = 'Remediate.ps1' + Before = "`$c = Import-Clixml C:\c.xml`n`$cred = Get-Credential -Credential `$c" + After = "`$c = Import-Clixml C:\c.xml`n`$cred = `$c" } + @{ Rule = 'IslExecutionPolicyCall'; Type = 'Remediation'; Name = 'Remediate.ps1' + Before = "Set-ExecutionPolicy Bypass -Scope Process -Force`nWrite-Output 'x'" + After = "`nWrite-Output 'x'" } + @{ Rule = 'IslArchitectureIssue'; Type = 'Remediation'; Name = 'Detect.ps1' + Before = 'Test-Path "$env:ProgramFiles\Widget\w.exe"' + After = 'Test-Path "$env:ProgramW6432\Widget\w.exe"' } + @{ Rule = 'IslArm64Assumption'; Type = 'Remediation'; Name = 'Detect.ps1'; Architecture = 'arm64' + Before = "if (`$env:PROCESSOR_ARCHITECTURE -match 'AMD64') { 'x64' }" + After = "if (`$env:PROCESSOR_ARCHITECTURE -match 'ARM64|AMD64') { 'x64' }" } + @{ Rule = 'IslOutputIssue'; Type = 'Win32Detection'; Name = 'Detect-App.ps1' + Before = 'Get-Item C:\Widget\w.exe' + After = 'Get-Item C:\Widget\w.exe -ErrorAction SilentlyContinue' } + @{ Rule = 'IslExitCodeIssue'; Type = 'Detection'; Name = 'Detect.ps1' + Before = "if (Test-Path C:\x) { exit 2 }`nexit 0" + After = "if (Test-Path C:\x) { exit 1 }`nexit 0" } + @{ Rule = 'IslRelativePath'; Type = 'Remediation'; Name = 'Remediate.ps1' + Before = 'Get-Content (Join-Path $PWD settings.json)' + After = 'Get-Content (Join-Path $PSScriptRoot settings.json)' } + @{ Rule = 'IslPowerShell7Syntax'; Type = 'Remediation'; Name = 'Remediate.ps1' + Before = "#Requires -Version 7.0`nWrite-Output 'x'"; After = "`nWrite-Output 'x'" } + ) { + # Each is the edit the finding's own message asks for, so the finding and its fix agree + $folder = if ($Type -eq 'Win32Detection') { 'Win32\M' } else { 'Remediations\M' } + $path = New-TestScript "$folder\$Name" $Before -Bom + $repairSplat = @{ Path = $path; ScriptType = $Type; IncludeRule = $Rule } + if ($Architecture) { $repairSplat.Architecture = $Architecture } + $result = Repair-IntuneScript @repairSplat + $result.Applied | Should-Be 1 + $result.Fixes[0].RuleName | Should-Be $Rule + [System.IO.File]::ReadAllText($path) | Should-Be $After + $testSplat = @{ Path = $path; ScriptType = $Type; IncludeRule = $Rule } + if ($Architecture) { $testSplat.Architecture = $Architecture } + @(Test-IntuneScript @testSplat | Where-Object { $_.Fix }).Count | Should-Be 0 + } + + It 'withholds a fix that would break the script: ' -ForEach @( + @{ Case = 'Set-ExecutionPolicy inside a pipeline'; Rule = 'IslExecutionPolicyCall' + Body = 'Set-ExecutionPolicy Bypass -Scope Process -Force | Out-Null' } + @{ Case = "'AMD64' compared with -eq"; Rule = 'IslArm64Assumption'; Architecture = 'arm64' + Body = "if (`$env:PROCESSOR_ARCHITECTURE -eq 'AMD64') { 'x64' }" } + @{ Case = '$PWD with a member after it'; Rule = 'IslRelativePath' + Body = 'Get-Content "$($PWD.Path)\s.json"' } + @{ Case = 'Set-ExecutionPolicy without -Force, which another rule removes'; Rule = 'IslInteractiveCall' + Body = 'Set-ExecutionPolicy RemoteSigned' } + ) { + $path = New-TestScript 'Remediations\N\Remediate.ps1' $Body -Bom + $testSplat = @{ Path = $path; ScriptType = 'Remediation'; IncludeRule = $Rule } + if ($Architecture) { $testSplat.Architecture = $Architecture } + $findings = @(Test-IntuneScript @testSplat) + $findings.Count | Should-BeGreaterThan 0 + @($findings | Where-Object { $_.Fix }).Count | Should-Be 0 + $result = Repair-IntuneScript @testSplat + $result.Applied | Should-Be 0 + [System.IO.File]::ReadAllText($path) | Should-Be $Body + } + } + Context 'Behaviour' { It 'changes nothing under -WhatIf but still reports what it would do' { $body = "if (Test-Path C:\x) { return 'ok' }`nexit 1" diff --git a/docs/IntuneScriptLab/Repair-IntuneScript.md b/docs/IntuneScriptLab/Repair-IntuneScript.md index 17636b0..4949872 100644 --- a/docs/IntuneScriptLab/Repair-IntuneScript.md +++ b/docs/IntuneScriptLab/Repair-IntuneScript.md @@ -43,9 +43,30 @@ behaviour-preserving edit: or an ANSI file (read in the system ANSI code page, so its characters survive) is rewritten as UTF-8 with a BOM, the encoding Intune expects IslOutputIssue a requirement script's output literal with leading or trailing - whitespace is trimmed, so it can match the portal value - -Everything else stays as it is and is counted in Remaining. + whitespace is trimmed, so it can match the portal value; a probing + cmdlet without -ErrorAction gets -ErrorAction SilentlyContinue, so a + missing target no longer writes to stderr + IslInteractiveCall Install-Module, Install-PackageProvider, Install-Package, Update-Module + and Uninstall-Module get -Force, Register-PSRepository gets + -Confirm:$false; a Get-Credential -Credential handed a credential that + can only be one already built is replaced by that credential + IslExecutionPolicyCall + a Set-ExecutionPolicy call that is a statement of its own is removed; + the agent launches the script with -ExecutionPolicy Bypass + IslExitCodeIssue 'exit N' with N other than 0 or 1 becomes 'exit 1', the value Intune + reads it as + IslArchitectureIssue + $env:ProgramFiles becomes $env:ProgramW6432, the 64-bit folder in + either host + IslArm64Assumption 'AMD64' as the pattern of a -match becomes 'ARM64|AMD64' + IslRelativePath $PWD becomes $PSScriptRoot where no member follows it + IslPowerShell7Syntax + a '#Requires -Version 7' line is removed; what the script then does + under Windows PowerShell 5.1, the other findings say + +Everything else stays as it is and is counted in Remaining. Where an edit would leave a broken +statement, Set-ExecutionPolicy inside a pipeline, 'AMD64' compared with -eq, $PWD.Path, the +finding has no fix and stays. Text edits are applied from the end of the file backwards so line numbers stay valid, line endings are kept, and an edit whose text no longer matches the file is skipped with a warning. diff --git a/en-US/IntuneScriptLab-Help.xml b/en-US/IntuneScriptLab-Help.xml index bd04a69..a21cce1 100644 --- a/en-US/IntuneScriptLab-Help.xml +++ b/en-US/IntuneScriptLab-Help.xml @@ -4169,9 +4169,30 @@ behaviour-preserving edit: or an ANSI file (read in the system ANSI code page, so its characters survive) is rewritten as UTF-8 with a BOM, the encoding Intune expects IslOutputIssue a requirement script's output literal with leading or trailing - whitespace is trimmed, so it can match the portal value + whitespace is trimmed, so it can match the portal value; a probing + cmdlet without -ErrorAction gets -ErrorAction SilentlyContinue, so a + missing target no longer writes to stderr + IslInteractiveCall Install-Module, Install-PackageProvider, Install-Package, Update-Module + and Uninstall-Module get -Force, Register-PSRepository gets + -Confirm:$false; a Get-Credential -Credential handed a credential that + can only be one already built is replaced by that credential + IslExecutionPolicyCall + a Set-ExecutionPolicy call that is a statement of its own is removed; + the agent launches the script with -ExecutionPolicy Bypass + IslExitCodeIssue 'exit N' with N other than 0 or 1 becomes 'exit 1', the value Intune + reads it as + IslArchitectureIssue + $env:ProgramFiles becomes $env:ProgramW6432, the 64-bit folder in + either host + IslArm64Assumption 'AMD64' as the pattern of a -match becomes 'ARM64|AMD64' + IslRelativePath $PWD becomes $PSScriptRoot where no member follows it + IslPowerShell7Syntax + a '#Requires -Version 7' line is removed; what the script then does + under Windows PowerShell 5.1, the other findings say -Everything else stays as it is and is counted in Remaining. +Everything else stays as it is and is counted in Remaining. Where an edit would leave a broken +statement, Set-ExecutionPolicy inside a pipeline, 'AMD64' compared with -eq, $PWD.Path, the +finding has no fix and stays. Text edits are applied from the end of the file backwards so line numbers stay valid, line endings are kept, and an edit whose text no longer matches the file is skipped with a warning.